Gizra / og

https://www.drupal.org/project/og
92 stars 133 forks source link

Og::invalidateCache() oversteps its responsibilities #219

Open pfrenssen opened 7 years ago

pfrenssen commented 7 years ago

It is expected that calling Og::invalidateCache() only invalidates the static caches of OG itself. However at the moment this method also clears caches in EntityTypeManager and EntityFieldManager:

    // Invalidate the entity property cache.
    \Drupal::entityTypeManager()->clearCachedDefinitions();
    \Drupal::service('entity_field.manager')->clearCachedFieldDefinitions();

This is probably intended to solve problems occurring after making changes to group audience fields, but this is way too drastic for most use cases and can have a negative performance impact. The entity type definitions and field definitions should only be cleared when strictly necessary.

amitaibu commented 7 years ago

Yeah, it probably related to the OG related field creation. Those should probably do the invalidations themself.

pfrenssen commented 5 years ago

Marking this as a novice issue. In order to fix this, here is a procedure you could follow:

  1. Remove the lines in Og::invalidateCache() that are clearing the entity type definition and field definition caches. They are marked with a @todo that links to this issue.
  2. Create a PR with the removal of these lines and check if any tests fail.
  3. If any tests fail, try to figure out which of the calls to Og::invalidateCache() is now missing an invalidation of the entity type definition or field definition cache. Add a line to invalidate the required cache following the call to Og::invalidateCache().
  4. Repeat until tests are green.