Updated: Comment #5

Problem/Motivation

Even internally the entity manager might be a plugin manager, it does provide anything you would expect it to be.

Proposed resolution

Keep the service as 'plugin.manager.entity', but provide an alias of 'entity.manager'.

Remaining tasks

N/A

User interface changes

N/A

API changes

API addition: plugin.manager.entity and entity_manager will retrieve the same service

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Title: Rename plugin.manager.entity » Provide an alias entity_manager for plugin.manager.entity.
Status: Active » Needs review
FileSize
1.53 KB

Damian had a wonderful idea!

damiankloip’s picture

+++ b/core/core.services.ymlundefined
@@ -160,6 +160,8 @@ services:
+  entity_manager:
+    alias: plugin.manager.entity

I would switch these round. /me hides

dawehner’s picture

FileSize
1.53 KB

With a dot.

damiankloip’s picture

Sorry, I still think this should be switched round, all other plugin managers are plugin.manager.xx, so I think entity manager should still have this as its primary, with 'entity.manager' as the alias.

tim.plunkett’s picture

#4, #3 does what you ask. I'll update the summary.

tim.plunkett’s picture

Issue summary: View changes

updated

tim.plunkett’s picture

Title: Provide an alias entity_manager for plugin.manager.entity. » Provide an alias for 'plugin.manager.entity' called 'entity.manager'
damiankloip’s picture

No, #3 adds plugin.manager.entity as the alias?

tim.plunkett’s picture

The one with the definition is the canonical one. That is still 'plugin.manager.entity'.
#3 adds a new service ID called 'entity.manager' that is an alias for 'plugin.manager.entity'.

See http://symfony.com/doc/current/components/dependency_injection/advanced...., the one with "alias: " is the alias.

damiankloip’s picture

Yep, you're totally right, sorry! I was looking at the patch on my mobile, which turns out doesn't help that much.

Status: Needs review » Needs work

The last submitted patch, drupal-2057869-3.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

#3: drupal-2057869-3.patch queued for re-testing.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityManagerTest.php
@@ -28,6 +28,7 @@ public static function getInfo() {
+    $this->assertEqual(spl_object_hash($entity_manager), spl_object_hash($this->container->get('entity.manager')));

Always like an spl_object_hash() comparison :)

Looks good, small and simple enough, and has test coverage!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

git ac https://drupal.org/files/drupal-2057869-3.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1564  100  1564    0     0   1600      0 --:--:-- --:--:-- --:--:--  2012
error: patch failed: core/core.services.yml:160
error: core/core.services.yml: patch does not apply
jibran’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
1.55 KB

Reroll.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 8c23138 and pushed to 8.x. Thanks!

dawehner’s picture

Should 'entity.manager' be the recommended way to access it?

yched’s picture

Should 'entity.manager' be the recommended way to access it?

Very much +1.
(copy/paste my usual rant that the EntityManager isn't a plugin manager)

dawehner’s picture

Status: Fixed » Needs review
FileSize
106.84 KB

Let's just also replace all instances.

Status: Needs review » Needs work

The last submitted patch, entity_manager-2057869-19.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
805 bytes
107.48 KB

Maybe the problem is http://stackoverflow.com/a/10985500/637596, but this fixes the problem.

jibran’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch doesn't apply anymore.

dawehner’s picture

Status: Needs work » Needs review
FileSize
107.08 KB

Just a rerole.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Yay.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Yet again...

git ac https://drupal.org/files/entity_manager-2057869-23.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  107k  100  107k    0     0  55035      0  0:00:01  0:00:01 --:--:-- 60379
error: patch failed: core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.php:72
error: core/modules/field_ui/lib/Drupal/field_ui/Form/FieldInstanceEditForm.php: patch does not apply
dawehner’s picture

Status: Needs work » Needs review
FileSize
106.95 KB

Just another rerole.

yched’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

:-)

jibran’s picture

Issue tags: +Avoid commit conflicts

Tagging. To avoid further rerolls.

dawehner’s picture

Issue tags: -Avoid commit conflicts

I really hate this tag personally, rerolling is simple.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Well the tag didn't get added and we need another reroll...

git ac https://drupal.org/files/entity_manager-2057869-26.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  106k  100  106k    0     0  49072      0  0:00:02  0:00:02 --:--:-- 53343
error: core/modules/layout/tests/layout_test/lib/Drupal/layout_test/Controller/LayoutTestController.php: does not exist in index
error: patch failed: core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php:69
error: core/modules/views_ui/lib/Drupal/views_ui/Controller/ViewsUIController.php: patch does not apply
dawehner’s picture

Status: Needs work » Needs review
FileSize
106.16 KB

.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

RTBC if green.

damiankloip’s picture

+ 1, for the 10th time :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

git ac https://drupal.org/files/entity_manager-2057869-31.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  106k  100  106k    0     0  55838      0  0:00:01  0:00:01 --:--:-- 61555
error: patch failed: core/modules/field_ui/lib/Drupal/field_ui/FieldListController.php:88
error: core/modules/field_ui/lib/Drupal/field_ui/FieldListController.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Views.php:85
error: core/modules/views/lib/Drupal/views/Views.php: patch does not apply
error: patch failed: core/modules/views/tests/Drupal/views/Tests/ViewsTest.php:50
error: core/modules/views/tests/Drupal/views/Tests/ViewsTest.php: patch does not apply
dawehner’s picture

Status: Needs work » Needs review
FileSize
106.15 KB

There we go.

613a370a5b4bbc22f07e1ac0b8ed87c5a0ecd030

jibran’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

And back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7742cc3 and pushed to 8.x. Thanks!

andypost’s picture

Status: Fixed » Needs review
FileSize
11 KB

This needs follow-up patch

damiankloip’s picture

Not in this issue, look at the title.

damiankloip’s picture

Status: Needs review » Fixed
andypost’s picture

damiankloip’s picture

Perfect.

jibran’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Update