Contrib.social

EntityViewsData assumes BaseFieldDefinitions where it should use FieldDefinitionInterface

Open on Drupal.org →
Created on 14 December 2017, about 7 years ago
Updated 30 January 2023, almost 2 years ago

Problem/Motivation

EntityViewsData has the following code around line 252:

    // Load all typed data definitions of all fields. This should cover each of
    // the entity base, revision, data tables.
    $field_definitions = $this->entityManager->getBaseFieldDefinitions($this->entityType->id());
    /** @var \Drupal\Core\Entity\Sql\DefaultTableMapping $table_mapping */
    if ($table_mapping = $this->storage->getTableMapping($field_definitions)) {

The problem is that it asks for FieldDefinitionInterface[] and immediately treats it as FieldStorageDefinitionInterface[] by calling $this->storage->getTableMapping() which expects the latter. Further down the code it correctly treats the retrieved data as FDI[] when it calls $this->mapFieldDefinition().

So why is this a problem? Because it is only working by happy accident. We know that we are getting BaseFieldDefinition objects, which we know implement both FDI and FSDI. The problem becomes clear when we try to patch EntityViewsData to actually take care of bundle fields as well, as demonstrated in 🐛 [PP-1] bundleFieldDefinitions() are not added in EntityViewsData Needs work

Consider the following code be inserted right after the setting of $field_definitions:

// Add any bundle fields defined in code.
if (isset($this->entityType->getKeys()['bundle'])) {
  $bundle_field_definitions = [];
  foreach ($this->entityManager->getBundleInfo($this->entityType->id()) as $bundle_id => $bundle_info) {
    $bundle_field_definitions += $this->entityManager->getFieldDefinitions($this->entityType->id(), $bundle_id);
  }
  $field_definitions += $bundle_field_definitions;
}

Now we might get BaseFieldDefinitions from the above, but we could also get FieldConfig items. Which is perfectly fine because both getFieldDefinitions() and getBaseFieldDefinitions() should return FDI[] and then be treated as such. Except, because we treat the returned data as both FDI[] and FSDI[], the above addition will lead to a crash because FieldConfig (among others) does not implement FSDI.

Proposed resolution

Properly treat the base field definitions as FDI[] only and rewrite EntityViewsData to properly retrieve the FSDI when needed.

Remaining tasks

  • Write a patch
  • Figure out whether this breaks BC

API changes

None, unless we have to change function signatures to ask for the right interface.

Data model changes

None.

Issue priority explanation

Marked as major because it is both a massive code smell and it blocks a useful patch that would allow bundle fields to show up in views.

🐛 Bug report
Status

Needs work

Version

10.1 ✨

Component
Views  →

Last updated about 7 hours ago

Created by

🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024