Split up EntityManager into many services

A service that handles fields on content entities... That sounds like going back to FieldInfo :-) (well, a richer version)

Not for or against atm, just saying.

There are parts of this that are internal-only, but some parts of this likely affect the public Entity API and should be beta deadline.

If this misses the beta deadline, presumably we could still create the new service and have the old methods just call it?

Hey, should I start to work on this ?

@prics
This would be cool. Maybe its worth to try to write it in a BC compatible way but I guess this API is not something every module would use ...

Actually, it is a pretty major API (change), so yes, I think we should keep the old functions around. We really can't just move on as we did before beta...

Related : #2472013: Move view_mode / display handilng code out of EntityManager and into a new EntityDisplayManager

Fun facts copied from there :

EntityManager has 1300 lines of code, 37 public methods, 27 "use" statements, 10 service dependencies (the first line of non-boilerplate code is at line 230 or so)

:-)

.

This all makes ton of sense for me. Naming also looks good. Here are few remarks I discussed in IRC with @tim.plunkett.

1. getTranslationFromContext has to move it has nothing to do with EntityManager. @tim.plunkett will ping @plach @GaborHojtsy about it.
2. EntityTypeRepository::loadEntityByUuid() & EntityTypeRepository::loadEntityByConfigTarget() seems out of place.
3. getTranslationFromContext, loadEntityByUuid, and loadEntityByConfigTarget all returns entity so @tim.plunkett thought we should move them to separate class. Class name is not yet decided. In theory getTranslationFromContext, loadEntityByUuid, and loadEntityByConfigTarget should exist on EntityStorageInterface with static method on EntityInterface just like ::load, ::create but @tim.plunkett said anything that implements EntityStorageInterface from scratch now has to rewrite those. I don't think moving those is a right move.
4. EntityDefinitionRepository should be renamed to InstalledEntityDefinitionRepository, EntityInstalledDefinitionRepository or EntityDefinitionInstalledRepository @tim.plunkett agreed to rename it to EntityInstalledDefinitionRepository.
5. We can add or remove from repo so I suggested s/deleteLastInstalledDefinition/removeLastInstalledDefinition and s/setLastInstalledDefinition/addLastInstalledDefinition on EntityDefinitionRepository. @tim.plunkett agrees but it would break the BC and we don't want that here.

Here is a code review.

1. +++ b/core/lib/Drupal/Core/Cache/CacheBackendTrait.php
   @@ -0,0 +1,52 @@
   +  protected function cacheGet($cid) {
   ...
   +  protected function cacheSet($cid, $data, $expire = Cache::PERMANENT, array $tags = []) {
   

   @param block missing.

2. +++ b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php
   @@ -7,318 +7,24 @@
   +interface EntityManagerInterface extends EntityTypeListenerInterface, EntityBundleListenerInterface, FieldStorageDefinitionListenerInterface, FieldDefinitionListenerInterface, EntityTypeManagerInterface, EntityTypeRepositoryInterface, EntityTypeBundleManagerInterface, EntityDisplayManagerInterface, EntityFieldManagerInterface {
   
   @@ -344,175 +50,17 @@ public function getExtraFields($entity_type_id, $bundle);
      public function getLastInstalledDefinition($entity_type_id);
   ...
   +  public function getLastInstalledFieldStorageDefinitions($entity_type_id);
   

   EntityDefinitionRepositoryInterface is missing from this interface. After adding that we can remove getLastInstalledDefinition and getLastInstalledFieldStorageDefinitions

3. +++ b/core/lib/Drupal/Core/Field/FieldDefinitionListener.php
   @@ -0,0 +1,154 @@
   +    if ($field_map) {
   ...
   +    $field_map = $this->entityFieldManager->getFieldMap();
   +    if ($field_map) {
   

   This can become single statement.

Pending tests review.

Looks good to me after removal of needs tag it is RTBC for me.

This looks ready. Just two minor points. I think CR should be in tabular form with before and after column in PHP code format so it would be easier to read. I can RTBC this but I don't think @Berdir, @yched and @plach would like that so let's wait for them to review it.

1. +++ b/core/lib/Drupal/Core/Entity/EntityTypeManager.php
   @@ -0,0 +1,266 @@
   +        // The entity manager cannot be injected due to a circular dependency.
   +        // @todo Remove this set call.
   +        ->setEntityManager(\Drupal::entityManager());
   

   Is this still necessary?

2. +++ b/core/tests/Drupal/Tests/Core/Entity/EntityFieldManagerTest.php
   @@ -0,0 +1,824 @@
   +        // @todo
   

   Stray @todo.

This will only disruptive for existing patches in IQ.

First: I really like that we finally split it up. It has the potential to also improve things a bit because I hope that most of those services aren't needed on runtime.

We should ensure that we don't have a worse performance after this rewrite.

Pointless interfaces in the sense that it doesn't tell you a single bit:

1. EntityDisplayManagerInterface
2. EntityFieldManagerInterface
3. EntityInstalledDefinitionRepository: So this here for example. What the doc could describe is that this concrete implementation uses KV in order to store the installed entity definitions, other implementations could use something else.
4. EntityRepositoryInterface
5. EntityTypeManagerInterface

Wrong things in general:

1. \Drupal\Core\Entity\EntityFormInterface::setEntityTypeManager ... the injection is an implementation detail, it doesn't belong onto the interface. You know an interface is about how an object interacts with some other object, the dependency injection is separate from that. No, an interface is not just a copy of all public methods. We are doing this so wrong all over the place in core, but well on the other hand it is great that we learn about proper software design over time.
2. The @deprecated in EntityForm/EntityFormInterface should describe when they are removed
3. \Drupal\Core\Entity\EntityTypeManagerInterface vs \Drupal\Core\Entity\EntityTypeRepositoryInterface This feels a bit redundant to be honest, just from the name. Would it make sense to have those two together?

   OH I see so the EntityTypeManager also takes care about the initialization of handlers, mhh the name doesn't tell me at all.

4. Nitpick:
   

      * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
      *   The entity type manager.
      * @param \Drupal\Core\Entity\EntityTypeRepositoryInterface $entity_type_repository
      *   The entity type repository.
      * @param \Drupal\Core\Entity\EntityTypeBundleManagerInterface $entity_type_bundle_manager
      *   The entity type bundle manager.
      * @param \Drupal\Core\Entity\EntityDisplayManagerInterface $entity_display_manager
      *   The entity display manager.
      * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
      *   The entity field manager.
      * @param \Drupal\Core\Entity\EntityTypeListenerInterface $entity_type_listener
      *   The entity type listener.
      * @param \Drupal\Core\Entity\EntityInstalledDefinitionRepositoryInterface $entity_installed_definition_repository
      *   The entity definition repository.
      * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
      *   The entity repository.
      * @param \Drupal\Core\Entity\EntityBundleListenerInterface $entity_bundle_listener
      *   The entity bundle listener.
      * @param \Drupal\Core\Field\FieldStorageDefinitionListenerInterface $field_storage_definition_listener
      *   The field storage definition listener.
      * @param \Drupal\Core\Field\FieldDefinitionListenerInterface $field_definition_listener
      *   The field definition listener.
      */

   Do you mind grouping them a bit better? So EntityRepositoryInterface is certainly more important. The two listeners could be next to each other.

5. So the @deprecated on EntityManager::() doesn't help you at all as developer. Alternative approach could be the following: Throw E_USER_DEPRECATION errors,
   and use #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest to handle Deprecation and some adaption in simpletest to collect the deprecated calls. Yes the support for deprecation will be hell during the entire release cycle.

6. * Note: The following interfaces that are extended are deprecated as of 8.0.0
    * and will be removed in 9.0.0:
    * - \Drupal\Core\Entity\EntityTypeManagerInterface
    * - \Drupal\Core\Entity\EntityTypeRepositoryInterface
    * - \Drupal\Core\Entity\EntityTypeBundleManagerInterface
    * - \Drupal\Core\Entity\EntityDisplayManagerInterface
    * - \Drupal\Core\Entity\EntityFieldManagerInterface
    * - \Drupal\Core\Entity\EntityTypeListenerInterface
    * - \Drupal\Core\Entity\EntityRepositoryInterface
    * - \Drupal\Core\Entity\EntityBundleListenerInterface
    * - \Drupal\Core\Field\FieldStorageDefinitionListenerInterface
    * - \Drupal\Core\Field\FieldDefinitionListenerInterface
   

   This sounds a bit wrong, it kinda tells me that the following interfaces will be removed in 9.0.0. You could then argue what is the point in adding them now :)

7. I'm a bit confused about the remaining methods on EntityManagerInterface, are you sure we still need them?
8. EntityRepositoryInterface: Not the problem of the patch but from the design of the software you would expect that you can load an entity via the repository. turns out, you can't :)
9. Why is EntityTypeBundleManagerInterface not named EntityBundleRepository or similar?
10. EntityTypeManagerTest: Small trick, you can actually namespace your module names, so you don't need to put those functions into global namespace but rather the one of the test

@tim

Re-rolled the patch & merged the changes from 8.0.x HEAD.

5) #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest isn't in yet, and we don't use E_USER_DEPRECATED anywhere else either. Not trying to make waves here...

What about adding a todo? These are all places we have to deal with at some point in time when we actually remove the old code.

See https://groups.drupal.org/node/484788 for more information on the rc target tag. This does sound like a potential RC target because of the minimal disruption, but tagging for a framework manager to evaluate that.

1. +++ b/core/lib/Drupal/Core/Entity/EntityDisplayManager.php
   @@ -0,0 +1,251 @@
   +class EntityDisplayManager implements EntityDisplayManagerInterface {
   

   The public methods for this look like a classical repository.

2. +++ b/core/lib/Drupal/Core/Entity/EntityFieldManagerInterface.php
   @@ -0,0 +1,148 @@
   +/**
   + * Provides an interface for an entity field manager.
   + */
   +interface EntityFieldManagerInterface {
   

   That one is tricky, Its almost like a EntityField(AndFieldDefinition)Listing

3. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleManagerInterface.php
   @@ -0,0 +1,39 @@
   + * Provides an interface for an entity type bundle manager.
   + */
   +interface EntityTypeBundleManagerInterface {
   +
   

   looking at the interface I would suggest something like EntityTypeBundleInformation?

4. +++ b/core/lib/Drupal/Core/Entity/EntityTypeManager.php
   @@ -0,0 +1,266 @@
   + */
   +class EntityTypeManager extends DefaultPluginManager implements EntityTypeManagerInterface, ContainerAwareInterface {
   +
   

   THe creation for the handlers could be moved into its own EntityHandlerFactory

5. +++ b/core/tests/Drupal/Tests/Core/Entity/EntityFieldManagerTest.php
   @@ -0,0 +1,824 @@
   +    $field_storage_definition = $this->prophesize(FieldStorageDefinitionInterface::class);
   +    $field_storage_definition->getName()->willReturn('field_storage');
   

   <3 <3 <3 <3

Overall: strong +1 for this direction.

1. +++ b/core/lib/Drupal/Core/Entity/EntityBundleListener.php
   @@ -0,0 +1,93 @@
   +class EntityBundleListener implements EntityBundleListenerInterface {
   

   This tripped me up, I wondered how it worked, but then I saw this in the interface:

   @todo Convert to Symfony events: https://www.drupal.org/node/2332935
   

   Okay, so this does not use Symfony events. Out of scope to fix here, of course. Hopefully me remarking this helps other reviewers save some time.

2. +++ b/core/lib/Drupal/Core/Entity/EntityFieldManager.php
   @@ -0,0 +1,601 @@
   + * Manages the discovery of entity fields.
   

   The word "discovery" in here confuses me. It's not like we have to "discover" which fields are defined on a certain entity type, we *know* that.

   Plugins are discovered, entity fields are not.

   Wouldn't it be better to just say Manages entity fields., and perhaps Manages entities fields (field definitions and their storage)..

3. +++ b/core/lib/Drupal/Core/Entity/EntityFieldManagerInterface.php
   @@ -0,0 +1,148 @@
   + * Provides an interface for an entity field manager.
   

   "an" implies there can be multiple operating at the same time?

4. +++ b/core/lib/Drupal/Core/Entity/EntityInstalledDefinitionRepository.php
   @@ -0,0 +1,97 @@
   + * Provides a repository for installed entity definitions.
   ...
   +class EntityInstalledDefinitionRepository implements EntityInstalledDefinitionRepositoryInterface {
   
   +++ b/core/lib/Drupal/Core/Entity/EntityInstalledDefinitionRepositoryInterface.php
   @@ -0,0 +1,130 @@
   + * Provides an interface for an installed entity definition repository.
   ...
   +interface EntityInstalledDefinitionRepositoryInterface {
   

   Why "installed"? Why not just EntityDefinitionRepository?

   Do we have entity definitions that are not installed?

   Perhaps the name is fine, but then I expect that the "installed" adjective is explained on the interface.

5. +++ b/core/lib/Drupal/Core/Entity/EntityRepository.php
   @@ -0,0 +1,120 @@
   + * Provides several mechanisms for retrieving entities.
   
   +++ b/core/lib/Drupal/Core/Entity/EntityRepositoryInterface.php
   @@ -0,0 +1,77 @@
   + * Provides an interface for an entity repository.
   

   I think the implementation's docblock's specificity belongs on the interface?

6. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleManager.php
   @@ -0,0 +1,133 @@
   + * Provides discovery and retrieval of entity type bundles.
   
   +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleManagerInterface.php
   @@ -0,0 +1,39 @@
   + * Provides an interface for an entity type bundle manager.
   

   Again, implementation's docblock is more helpful than the interface's.

Also: EntityFieldManagerInterface vs. EntityInstalledDefinitionRepository, even they offer very similar functionality/data, the former for fields, the latter for entities.

Shouldn't their interfaces/services then also have similar names? EntityFieldDefinitionRepository and EntityDefinitionRepository would make sense from my non-deep-Entity/Field-API-knowledge POV :)

s/EntityInstalledDefinitionRepository/EntityDefinitionRepository

Sorry wim but this change is just fundamental wrong. If you look at the methods this is ALL about the installed definitions, which clearly makes a difference compared to the entity definitions you can retrieve directly from the entity manager.

s/EntityFieldManager/EntityFieldDefinitionRepository

Mh, its not just about field definitions if you are honest about it.

Daniel:

would make sense from my non-deep-Entity/Field-API-knowledge POV :)

Well, just saying :)

Agreed that EntityDefinitionRepository having a method to get FieldStorages (getLastInstalledFieldStorageDefinitions()) is a bit confusing when there is a EntityFieldDefinitionRepository next to it.

The methods currently grouped in EntityDefinitionRepositoryInterface (former EntityInstalledDefinitionRepositoryInterface) are about getting the "last installed" definitions for things (entity definitions and field definitions) whose "current" (usual) definitions are normally handled by two separate repositories (resp. EntityManager and EntityFieldDefinitionRepository)

So the only thing that "groups" those otherwise separate methods is the "last installed" aspect (which, I agree, is not too intuitive off-hand) :-)

If I'm not mistaken, "last installed" is to be understood with respect to the schema, right ? So maybe EntityLastInstalledSchemaRepository Interface would be slightly clearer than just EntityInstalledDefinitionRepositoryInterface ?

wow, impressive work + patch. Generally, big +1 on the approach - EM really needs to be split up. Given the BC-layer I do not see a problem with it doing that after RC also.

ad EntityFieldDefinitionRepository

Good it's gone, I do not like using repository for field definition.s Afaik Repository is mostly used for handling persistence elsewhere, and as field definitions are generally not persisted this would not be a good fit imho.

Tagging for triage per #50. Sorry it's taking a while to actually get to the triaging.

LOL. Just as I wrote #65, @catch and I finished discussing this and decided to approve this for RC, so retagging. Note that when one of us reviews the patch itself, we may have feedback, but in principle, having different interfaces and different service IDs for the different subparts, while keeping the giant interface and service ID for BC, seems low disruption and a good benefit to contrib modules that need to do significant interaction with subparts of entity management.

Just some more review`.

1. +++ b/core/core.services.yml
   @@ -495,10 +495,37 @@ services:
   +  entity_field.manager:
   ...
   +    arguments: ['@entity_type.manager', '@entity_type.bundle.info', '@entity_display.repository', '@typed_data_manager', '@language_manager', '@keyvalue', '@module_handler', '@cache.discovery']
   

   Its sad how many dependencies this particular service has

2. +++ b/core/lib/Drupal/Core/Cache/CacheBackendTrait.php
   @@ -0,0 +1,80 @@
   +
   +/**
   + * Provides methods to use cache backend while respecting a 'use caches' flag.
   + */
   +trait CacheBackendTrait {
   

   I'm curious whether we could/should reflect this flag on the cache backend. For some reason I would also expect an 'a' somewhere in this sentence.

3. +++ b/core/lib/Drupal/Core/Entity/EntityBundleListener.php
   @@ -0,0 +1,93 @@
   +class EntityBundleListener implements EntityBundleListenerInterface {
   

   So yeah thinking more about it, it maybe should be named EntityBundleCrudListenerInterface and similar in the first place?

   Btw. according to the interface its a CD only

4. +++ b/core/lib/Drupal/Core/Entity/EntityFieldManager.php
   @@ -0,0 +1,601 @@
   +/**
   + * Manages the discovery of entity fields.
   + *
   + * This includes field definitions, base field definitions, and field storage
   + * definitions.
   + */
   

   I like this bit of documentation

5. +++ b/core/lib/Drupal/Core/Entity/EntityTypeListener.php
   @@ -0,0 +1,121 @@
   +class EntityTypeListener implements EntityTypeListenerInterface {
   

   I'm curious whether we can find a better name to something more selfdescriptive, like EntityTypeCrudListener

6. +++ b/core/lib/Drupal/Core/Entity/EntityTypeRepository.php
   @@ -0,0 +1,113 @@
   +/**
   + * Provides helper methods for loading entity types.
   + *
   

   This itself seems way more useful than the docs on the interface to be honest. Keep in mind that people see the interface only

7. +++ b/core/lib/Drupal/Core/Entity/EntityTypeRepository.php
   @@ -0,0 +1,113 @@
   +  public function getEntityTypeFromClass($class_name) {
   +    // Check the already calculated classes first.
   +    if (isset($this->classNameEntityTypeMap[$class_name])) {
   +      return $this->classNameEntityTypeMap[$class_name];
   +    }
   +
   

   OT: Just in general, Its crazy that we don't store that information but rather determine this always on runtime

8. +++ b/core/lib/Drupal/Core/Entity/EntityTypeRepositoryInterface.php
   @@ -0,0 +1,59 @@
   +/**
   + * Provides an interface for an entity type repository.
   + */
   +interface EntityTypeRepositoryInterface {
   

   Can we put anything online there?

9. +++ b/core/lib/Drupal/Core/Field/FieldDefinitionListener.php
   @@ -0,0 +1,152 @@
   +/**
   + * Reacts to field definition CRUD on behalf of the Entity system.
   + */
   
   +++ b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php
   @@ -0,0 +1,124 @@
   +/**
   + * Reacts to field storage definition CRUD on behalf of the Entity system.
   + */
   +class FieldStorageDefinitionListener implements FieldStorageDefinitionListenerInterface {
   

   Can we put an @see to the event classes to connect things a little bit more here?

10. +++ b/core/lib/Drupal/Core/Field/FieldDefinitionListener.php
    @@ -0,0 +1,152 @@
    +class FieldDefinitionListener implements FieldDefinitionListenerInterface {
    

    Some question regarding the naming ... just to be clear, the classname doesn't always have to follow the interface strictly ... that is the entire point of having an interface

11. +++ b/core/lib/Drupal/Core/Field/FieldStorageDefinitionListener.php
    diff --git a/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
    index 5b7b86b..8d158e9 100644
    
    index 5b7b86b..8d158e9 100644
    --- a/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
    
    --- a/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
    +++ b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
    
    +++ b/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php
    @@ -8,13 +8,14 @@
    +use Drupal\Core\Cache\CacheBackendInterface;
    +use Drupal\Core\Cache\CacheBackendTrait;
    

    Ideally we would solve that in its own issue, it just feels weird to change DefaultPluginManager here as well

Can we put anything online there?

That was probably "useful"

11) I'd split it out to another issue if we weren't dealing with rc target stuff...

Are we sure that this is a valid argument when it comes to rc target stuff? Dont' we still want small reviewable pieces so we can even much better see errors before they come up unlike what we do now with one big big patch.

I had one question ... why we still have methods on the entity manager interface, but well, this is due to the problem that the EntityLastInstalledSchemaRepositoryInterface has more methods, so letting EMInterface extend that would actually be a BC break.

In general this patch is a really great step towards a more sane future

Given that catch is a entity subsystem maintainer I think this could be marked as RTBC

Assigning to catch since catch is both a committer and an entity subsystem maintainer

OK didn't review the whole patch and in fact tried not to since a lot of it is just moving code.

However found a few things that both looked new and I had questions about.

Unassigning me, I'm happy for this to go in, makes things so much clearer where the dependencies are, even if have minor issues with the stuff below.

+++ b/core/core.services.yml
--- /dev/null
+++ b/core/lib/Drupal/Core/Cache/CacheBackendTrait.php

The classname and location makes it look like a trait that cache backends would use, but instead it's a trait for using cache backends in a certain way - skipping caching.

Could use more documentation on when to use it, and possibly a rename.

1. +++ b/core/lib/Drupal/Core/Cache/CacheBackendTrait.php
   @@ -0,0 +1,80 @@
   +   * Flag whether persistent caches should be used.
   

   There's no static caching here, so it's more whether the cache should be read from or written to at all.

   Also the API here is incomplete, reading it I'd expect it to handle other CacheBackendInterface methods.

2. +++ b/core/lib/Drupal/Core/Entity/EntityFieldManagerInterface.php
   @@ -0,0 +1,148 @@
   +  /**
   +   * Disable the use of caches.
   +   *
   +   * @param bool $use_caches
   +   *   FALSE to not use any caches.
   +   *
   +   * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
   +   */
   +  public function useCaches($use_caches = FALSE);
   

   Why does the deprecated method have to be on the brand new interface? Couldn't we leave that one on EntityManager for bc only?

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -7,1468 +7,512 @@
    
   -use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
   -use Drupal\Component\Plugin\Exception\PluginNotFoundException;
   -use Drupal\Core\Cache\Cache;
   -use Drupal\Core\Cache\CacheBackendInterface;
   -use Drupal\Core\Config\Entity\ConfigEntityType;
   -use Drupal\Core\DependencyInjection\ClassResolverInterface;
   -use Drupal\Core\Entity\Exception\AmbiguousEntityClassException;
   -use Drupal\Core\Entity\Exception\InvalidLinkTemplateException;
   -use Drupal\Core\Entity\Exception\NoCorrespondingEntityClassException;
   -use Drupal\Core\Extension\ModuleHandlerInterface;
   -use Drupal\Core\Field\BaseFieldDefinition;
    use Drupal\Core\Field\FieldDefinitionInterface;
   -use Drupal\Core\Field\FieldStorageDefinitionEvent;
   -use Drupal\Core\Field\FieldStorageDefinitionEvents;
    use Drupal\Core\Field\FieldStorageDefinitionInterface;
   -use Drupal\Core\Field\FieldStorageDefinitionListenerInterface;
   -use Drupal\Core\KeyValueStore\KeyValueFactoryInterface;
   -use Drupal\Core\Language\LanguageInterface;
   -use Drupal\Core\Language\LanguageManagerInterface;
   -use Drupal\Core\Plugin\DefaultPluginManager;
   -use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery;
   -use Drupal\Core\StringTranslation\StringTranslationTrait;
   -use Drupal\Core\StringTranslation\TranslationInterface;
   -use Drupal\Core\TypedData\TranslatableInterface;
   -use Drupal\Core\TypedData\TypedDataManagerInterface;
   

   Look at that <3

4. +++ b/core/lib/Drupal/Core/Entity/EntityTypeManager.php
   @@ -0,0 +1,266 @@
   +        // @todo Remove this set call.
   

   Could use a bit more context.

5. +++ b/core/lib/Drupal/Core/Entity/EntityTypeRepositoryInterface.php
   @@ -0,0 +1,59 @@
   +   * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
   

   Same question with the deprecation - why not leave this in EntityManager and not bring it over here?

Ticking credit boxes for @dawehner and @Wim Leers cause @tim.plunkett asked me to.

Committed f0928e5 and pushed to 8.0.x. Thanks!

I found this whole new methodology for querying entities confusing. I finally got my solution working, but I decided to write up a more detailed description of what I did in case anyone else needs it. C&C welcome.

https://www.drupal.org/node/2549139#comment-13381841