Contrib.social

EntityViewsData cannot be reliably built twice in the same request if base fields change

Open on Drupal.org →
Created on 3 May 2017, over 7 years ago
Updated 31 January 2023, almost 2 years ago

Problem/Motivation

It is possible to end up with invalid views data if caches get built twice in a single request. This can cause install errors where config dependencies are concerned. NB: This only occurs when base fields change outside of a module install, e.g. config change.

  1. Calls ViewsData::clear - ViewsData::getAll builds the data rather than retrieve from cache.
  2. Calls ViewsData::getAll - collects views data, including constructing all ‘views_data’ handlers, eg. UserViewsData.
  3. Add a base field to an entity - calls ViewsData::clear via views_field_config_insert as the views data is now incomplete.
  4. ViewsData::getAll - collects data, including already instantiated ‘views_data’ handlers.

The second ViewsData::getAll ends up with incomplete field storage definitions, because the already instantiated UserViewsData uses a version of the field storage definitions from the first run through prior to the new field being added (see \Drupal\views\EntityViewsData::getFieldStorageDefinitions).

Example

Contacts module adds a profile type, an entity reference field to that profile type on a user and a view that uses that field. ProfileType::postSave rebuilds the router, which triggers ViewsData::getAll. The field is then added and then a view that depends on it. When the view is added, ViewsData::getAll is triggered and does not return a required field. This causes an install exception. #2875157: ProfileType::postSave should not immediately rebuild the router → is looking into a possibly inappropriate rebuild (rather than setRebuildNeeded), but this may be something that should also be addressed in core, as even if this case can be avoided it still feels fragile.

Possible resolutions

  1. Don’t store a cache of the field data in EntityViewsData::$fieldStorageDefinitions. Possible performance implications?
  2. Add a clear method to EntityViewsData that is triggered by ViewsData::clear. Will result in lot more work every time ViewsData::clear is called.
  3. Use a fresh instance of the handler each time. Will perform identically in almost all scenarios, but will ensure every time ViewsData::getAll is called we have clean information from entity ‘views_data’ handlers. Essentially just switching views_views_data to use EntityTypeManager::createHandlerInstance rather than EntityTypeManager::getHandler.

Remaining tasks

  • Write a test only patch that demonstrates the issue. Kernel test depending on views and user where we do the problem steps and check the new field definition exists at the end.
  • Write a patch that fixes - I would suggest solution 3 as I think it’s the best.
🐛 Bug report
Status

Needs work

Version

10.1 ✨

Component
Views  →

Last updated about 3 hours ago

Created by

🇬🇧United Kingdom yanniboi UK

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

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