Problem/Motivation

Discovered in farmOS: https://github.com/farmOS/farmOS/issues/1090

Summary: We haven't been writing update hook when we made changes to the Log entity type definition, so the cached definition in the key_value table may be outdated.

It seems that this doesn't matter in most contexts, but can cause errors in others (like in the GitHub issue linked above).

Proposed resolution

We should review all changes that have been made to the Log entity type definition since v2 was released and provide an update hook that ensures they are reflected in the cached key_value entity type definition.

We should also make sure that update hooks are added in any future changes.

Remaining tasks

  • Review changes to the entity type definition since v2.
  • Write an update hook that ensures those changes are reflected in the cached key_value entity type definition.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork log-3604200

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:

About issue forks

Comments

m.stenta created an issue. See original summary.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

Looking at the history of changes to src/Entity/Log.php, there are a lot of changes... but I think we only need to focus on ones that have happened since the release of Log 2.0.0 (18 Oct 2021), since that is when we officially started supporting Drupal 8+. Furthermore, we only need to focus on changes to the ContentEntityType annotation/attribute properties. That narrows it down to the following changes, which are all relatively recent (26 Jan 2024):

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Related issues: +#3600574: Use Drupal core RevisionHtmlRouteProvider

Notably, this one already included an update hook, added in #3600574: Use Drupal core RevisionHtmlRouteProvider.

m.stenta opened merge request !33

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Issue summary: View changes
Status: Active » Needs review
m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs review » Needs work

I removed this update because I don't think it actually works:

/**
 * Update log entity revision route provider.
 */
function log_post_update_revision_route_provider(&$sandbox) {
  /** @var \Drupal\Core\Entity\EntityDefinitionUpdateManagerInterface $manager */
  $manager = \Drupal::service('entity.definition_update_manager');
  $entity_type = $manager->getEntityType('log');
  $route_providers = $entity_type->getRouteProviderClasses();
  $route_providers['revision'] = RevisionHtmlRouteProvider::class;
  $entity_type->set('route_providers', $route_providers);
  $manager->updateEntityType($entity_type);
}

I'm not what the correct way to update route_providers is... I'll see if I can find some precedent for this...

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

FWIW I tested all the other updates and they work as expected.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Needs work » Needs review

I'm not what the correct way to update route_providers is...

I figured this out and restored the update hook for it.

It just needed to be $entity_type->setHandlerClass('route_provider', $route_providers); instead of $entity_type->set('route_providers', $route_providers);.

Tested and it works as expected.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented

@pcambra in chat:

I normally do instanceof instead of the variable type hinting so I can check the object AND get the IDE understading whats going on
otherwise the change seems sensible

https://irc.farmos.org/bot/log/farmOS/2026-06-19#T104290