Replace deprecated entityManager() in ControllerBase descendents

#2624770: Use more specific entity.manager services in core.services.yml seems to do a lot of it already ...

@dawehner
You are right, the patch in #2624770: Use more specific entity.manager services in core.services.yml already updates a lot of deprecated code, but it is focused specifically on services definitions and constructors and it is already enormous… And at the moment of this writing, needs a very big rebase.

On the contrary, this issue can focus specifically on usages of deprecated entityManager() method in controller classes, which is very common across all modules in core.

On the contrary, this issue can focus specifically on usages of deprecated entityManager() method in controller classes, which is very common across all modules in core.

Thank you for adding a good scope for this issue. Good point! Let's focus on this one thing here. This makes it easy to review and by that actually its doable to get in. Sorry for my previous comment.

I think that's all of them...

I added a follow-up #2914956: Remove usages of EntityManager from EntityViewController because NodeController uses NodeViewController to render some output.

Reroll, fixing the quickedit controller and also converting one more example from book.module.

I think it makes sense to deal with Node/EntityViewController in here too, it's easy to update and backwards compatible for sub-classes (once we add the new trait, which I'll do in the next update). Then we can add a @trigger_error() to ControllerBase::getEntityManager()

Fixed that.

Actually fixing the quickedit controller.

Reroll, updating BC handling.

Added missing constructor argument.

Looks good to me

+++ b/core/modules/node/src/Controller/NodePreviewController.php
@@ -35,7 +35,7 @@ public function view(EntityInterface $node_preview, $view_mode_id = 'full', $lan
    */
   public function title(EntityInterface $node_preview) {
-    return $this->entityManager->getTranslationFromContext($node_preview)->label();
+    return $this->entityTypeManager->getTranslationFromContext($node_preview)->label();
   }
 

this is wrong. apparently this method is not actually called in core.

this should fix that.

Good catch
Can you open an issue for removing it/finding out what title callback is used on that route?

The title callback does point to that method, but that specific route sets a #title (through EntityViewBuilder); so going to that URL does not have to call the title callback. And nothing references that route in a way that requires us to resolve the title through the callback (e.g. breadcrumb), not in core anyway, so it's never called.

It possibly was called at some point, but then EntityViewController was refactored to set the #title through the render-trickery, so that quickedit works. Note that it actually can work on the preview page :)

So, not quite sure what to do about that..

Created #3024386: Replace title callback on node preview route with a static title

+++ b/core/modules/comment/src/Controller/CommentController.php
@@ -53,13 +62,31 @@ class CommentController extends ControllerBase {
+    if ($entity_field_manager) {
+      $this->entityFieldManager = $entity_field_manager;
+    }
+    else {
+      @trigger_error('The entity_field.manager service must be passed to CommentController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityFieldManager = \Drupal::service('entity_field.manager');
+    }
+    if ($entity_repository) {
+      $this->entityRepository = $entity_repository;
+    }
+    else {
+      @trigger_error('The entity.repository service must be passed to CommentController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityRepository = \Drupal::service('entity.repository');
+    }

+++ b/core/modules/content_translation/src/Controller/ContentTranslationController.php
@@ -24,21 +25,40 @@ class ContentTranslationController extends ControllerBase {
+    if ($entity_field_manager) {
+      $this->entityFieldManager = $entity_field_manager;
+    }
+    else {
+      @trigger_error('The entity_field.manager service must be passed to ContentTranslationController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityFieldManager = \Drupal::service('entity_field.manager');
+    }

+++ b/core/modules/image/src/Controller/QuickEditImageController.php
@@ -50,11 +58,20 @@ class QuickEditImageController extends ControllerBase {
+    if (!$entity_display_repository) {
+      @trigger_error('The entity_display.repository service must be passed to QuickEditImageController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityDisplayRepository = \Drupal::service('entity_display.repository');
+    }
+    else {
+      $this->entityDisplayRepository = $entity_display_repository;
+    }

+++ b/core/modules/node/src/Controller/NodeController.php
@@ -39,10 +47,19 @@ class NodeController extends ControllerBase implements ContainerInjectionInterfa
+    if ($entity_repository) {
+      $this->entityRepository = $entity_repository;
+    }
+    else {
+      @trigger_error('The entity.repository service must be passed to NodeController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityRepository = \Drupal::service('entity.repository');
+    }

+++ b/core/modules/node/src/Controller/NodePreviewController.php
@@ -2,14 +2,57 @@
+    if ($entity_repository) {
+      $this->entityRepository = $entity_repository;
+    }
+    else {
+      @trigger_error('The entity.repository service must be passed to NodePreviewController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityRepository = \Drupal::service('entity.repository');
+    }

+++ b/core/modules/quickedit/src/QuickEditController.php
@@ -61,12 +77,30 @@ class QuickEditController extends ControllerBase {
+    if (!$entity_display_repository) {
+      @trigger_error('The entity_display.repository service must be passed to QuickEditController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityDisplayRepository = \Drupal::service('entity_display.repository');
+    }
+    else {
+      $this->entityDisplayRepository = $entity_display_repository;
+    }
+    if ($entity_repository) {
+      $this->entityRepository = $entity_repository;
+    }
+    else {
+      @trigger_error('The entity.repository service must be passed to QuickEditController::__construct(), it is required before Drupal 9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      $this->entityRepository = \Drupal::service('entity.repository');
+    }

I would turn these around to be

if (!$service) {
  $service = \Drupal::service('blah')
}
$this->service = $service;

Because then in the future when we remove the deprecation we have less change to make and less code moves... i.e. we can remove the entire if and the = NULL from the constructor and we are done.

There might be a compelling reason to do it the way round @Berdir has - if there is I'm +1 for re-rtbc :)

I don't have a reason, I think I copied that pattern in #2624770: Use more specific entity.manager services in core.services.yml from somewhere. Will have to update a lot of cases for that, but there are worse things ;)

Updated that, also realized I missed one controller when adding the BC changes, changed that now to just use entityTypeManager() like it used entityManager() before.

Thanks for the quick fixes. Marking rtbc as the changes are minimal. I've review all the service names in the deprecated code paths and they look good.

Committed c23c05c and pushed to 8.7.x. Thanks!