Problem/Motivation

Currently, \Drupal\Core\Entity\EntityBase::getTypedData() does not consider the derivatives generated by \Drupal\Core\Entity\Plugin\DataType\Deriver\EntityDeriver. Instead, an EntityAdapter is returned for all entities (e.g., instead of a ConfigEntityAdapter for configuration entities).

Proposed resolution

EntityBase::getTypedData() should be updated to consider all entity data type derivatives - not only for entity types, but also entity type bundles (e.g., entity:node and entity:node:article).

Doing so will cause the correct typed data object to be returned by EntityBase::getTypedData().

Remaining tasks

RTBC & commit

Issue fork drupal-2870874

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

clayfreeman’s picture

Since a ConfigEntityAdapter has been added, I'm going to be investigating a path to making sure it's returned by ::getTypedData().

tim.plunkett’s picture

clayfreeman’s picture

Status: Active » Needs review

The MR from #11 modifies \Drupal\Core\Entity\EntityBase::getTypedData() to use derivative data types for entities where possible. As a result, the correct entity adapter should be returned according to the logic in \Drupal\Core\Entity\Plugin\DataType\Deriver\EntityDeriver::getDerivativeDefinitions():

      $class = $entity_type->entityClassImplements(ConfigEntityInterface::class) ? ConfigEntityAdapter::class : EntityAdapter::class;

I added a test case for this change to \Drupal\Tests\Core\Entity\ContentEntityBaseUnitTest since EntityBase didn't seem to have its own unit test, and it would be much more involved to not leverage existing testing infrastructure in this unit test. Let me know if a unit test for EntityBase should be created.

clayfreeman’s picture

clayfreeman’s picture

Title: Figure out what to do with ConfigEntityBase::getTypedData() » EntityBase::getTypedData() does not return the correct data type for configuration entities due to lack of consideration for data type derivatives
Issue summary: View changes

Per @alexpott's review via Slack:

The issue summary could do with a bit more info as to why this needs to be done (less reading of tea leaves) and I think it’d be good to have some testing outside of unit tests as the existing test is full of assumptions based on mocks.

I've taken the following actions:

  1. Update the issue summary to accurately reflect the goal of this issue
  2. Add a kernel test to assert the intended behavior of ::getTypedData()

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tim.plunkett’s picture

+1 to this improvement, nice work @clayfreeman.

clayfreeman’s picture

Status: Needs review » Reviewed & tested by the community

This looks great - thanks Tim!

clayfreeman’s picture

Actually, I noticed the new test case was failing due to the change in logic introduced in #16, so I adjusted the test to match the changes therein.

Since the scope of the change is fairly small, I don't believe it warrants demotion from RTBC.

Wim Leers’s picture

Really nice work here, but it currently does not yet work for config entity types. I know config entity types do not yet have validation constraints in Drupal core, but this is possibly the last missing piece before those can "just work". See my comments on the MR. This would help #2164373: [META] Untie config validation from form validation — enables validatable Recipes, decoupled admin UIs … … and getting CKEditor 5 in core (which will use config entity validation).

Wim Leers’s picture

Ugh, never mind, the suggestion I left on the MR won't work, because apparently the config schema structure (filter.format.*) does not match the pre-existing data type definition for config entity types (which indeed is entity:filter_format, which the current merge request is already generating).

Either way, this issue is a step forward for making config entity validation possible, because $filter_format->getTypedData()->validate() now returns ConfigEntityAdapter instead of EntityAdapter :)

RTBC++

clayfreeman’s picture

Category: Task » Bug report
larowlan’s picture

Status: Reviewed & tested by the community » Needs review

Left some comments on the MR.

Technically this was self-RTBC'd by @clayfreeman which is frowned upon.

But it was also RTBC+1 from Wim, which makes it OK I think.

Because my comments on the MR don't change the fundamentals, its ok to re-rtbc this after those changes.

clayfreeman’s picture

Status: Needs review » Reviewed & tested by the community

I opted to rename the ::getClass() method to ::getTypedDataClass() since the original name wasn't super descriptive of what the method was doing.

Re-RTBC per #22. (Original RTBC was for Tim's changes prior to #16.)

  • larowlan committed 61bff4e on 9.3.x
    Issue #2870874 by clayfreeman, tim.plunkett, Wim Leers, larowlan,...

larowlan’s picture

Title: EntityBase::getTypedData() does not return the correct data type for configuration entities due to lack of consideration for data type derivatives » [backport] EntityBase::getTypedData() does not return the correct data type for configuration entities due to lack of consideration for data type derivatives
Version: 9.3.x-dev » 9.2.x-dev

Committed 61bff4e and pushed to 9.3.x. Thanks!

I think there's a slight risk of disruption here so erring against backporting to 9.2.x

I'll ask for a second opinion.

clayfreeman’s picture

Pushed the patchset rebased on 9.2.x - let me know if adjustments need to be made.

larowlan’s picture

Title: [backport] EntityBase::getTypedData() does not return the correct data type for configuration entities due to lack of consideration for data type derivatives » EntityBase::getTypedData() does not return the correct data type for configuration entities due to lack of consideration for data type derivatives
Version: 9.2.x-dev » 9.3.x-dev
Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed

In discussing the potential impact of this w.r.t its eligibility for backport, I'm of the opinion that this is more of a part feature request, part bug, as there is nothing in core that implements ::createFromEntity for anything other than the base EntityAdapter.

So I'm going to split the difference and mark this as a task. I'll add a change record detailing the new functionality available to those who alter the typed-data plugin definitions.

larowlan’s picture

Status: Fixed » Closed (fixed)

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