Problem/Motivation

\Drupal\Core\Entity\EntityManager uses the discovery method \Drupal\Core\Plugin\DefaultPluginManager sets. This includes support for derivers, but entity types cannot be derived, as colons in entity type IDs are not allowed due to restrictions in config storage and route requirements where colons are disallowed.

Proposed resolution

Explicitly remove support for derivers.

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#1 drupal_2470924_1.patch1.75 KBxano
#1 drupal_2470924_proof_of_bug-do-not-test.patch491 bytesxano

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Active » Needs review
StatusFileSize
new drupal_2470924_proof_of_bug-do-not-test.patch491 bytes
new drupal_2470924_1.patch1.75 KB

Here are a patch that expose the fact that derivers are run, and a fix for the bug.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

This is another example of where #2422815: Don't initialize the discovery object in plugin managers, unless needed would be nice

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Also, DerivativeDiscoveryDecorator::mergeDerivativeDefinition() will not work with non-array definitions.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

As mentioned on IRC, #2458789: Introduce PluginDefinitionInterface can introduce support for 'internal' definition merges. This means that theoretically we can re-introduce derivative support to entities in the future.

I discussed this issue with berdir at Drupal Dev Days and our conclusion was to just remove the illusion of support for now and get this issue in.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

I think this makes sense, making it look like derivatives work is currently misleading and they would be very broken. We can still make it nicer in the other issue, it doesn't have to block this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed a336347 and pushed to 8.0.x. Thanks!

  • alexpott committed a336347 on 8.0.x 
    Issue #2470924 by Xano: Entity manager allows entity type derivers, but...

Status: Fixed » Closed (fixed)

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