I noticed that during updating a node, pathauto checks for usable patterns before the node is updated in the database. That means if it ever compares entity field values, it doesn't use the updated ones.
How I discovered that:
PathautoWidget calls $pattern = \Drupal::service('pathauto.generator')->getPatternByEntity($entity); and the usable patterns are saved to $patterns property in PathautoGenerator. But the $entity parameter here doesn't contain the new field values, yet.
Later, in pathauto_entity_update hook, the \Drupal::service('pathauto.generator') is called again and there is no check for possible patterns, the "cached" $patterns property value is used. Shouldn't the module check for patterns only now that the new field values are available?
There's one case I encountered where this causes problems: with manually programmed custom conditions added to a pattern. Conditions like checking if a field has a specific value.
For example:
- Pattern1 is used for article node & when field_category = Books
- Pattern2 is used for article node & when field_category = E-Books
What happens in the example is:
- when updating article node1 I change the field_category to E-Books from Books
- after I click save, the alias is not updated, it still uses Pattern1 (because when the getPatternByEntity function checked patterns, it didn't use the newly saved node, and field_category was still Books)
- if I open node1 and click save again without changing anything, the alias is updated using Pattern2 (because when the getPatternByEntity function checked patterns, field_category was E-Books before and after saving the node)
Reset the cached pattern for the entity before updating its alias.
Cover this case with tests?
No.
No.
No.
Show commandsStart 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:
changes
Comments
Comment #4
dmitriy.trt commentedCreated MR with a fix.
Comment #5
berdirMakes sense, but the static cache is pretty pointless then at that point, no?
Comment #6
bkosborneNot entirely.
->updateEntityAliascalls->createEntityAlias()which checks the static cache again, which is now re-hydrated. Also->getPatternByEntity()is public and used in a few areas that benefit from this cache. It's worth keeping the static cache around, it just needs to be cleared here because the form widget hydrates the cache initially before the entities data is fully available.This seems straightforward to me and would help resolve issues surfaced in #3224916: Provide UI for pattern conditions. That issue makes it much easier to add additional conditions to a pattern, which opens the door for making conditions based on entity data.
Comment #7
bkosborneComment #9
mably commentedSummary
Added a
resetEntityPatternCache()method toPathautoGeneratorand a call to it at the top ofupdateEntityAlias(), so that the per-entity cached pattern is cleared beforegetPatternByEntity()re-evaluates which pattern applies. This prevents stale cached patterns from being reused when an entity's values have changed in a way that would match a different pattern.Changes in
src/PathautoGenerator.php:resetEntityPatternCache(EntityInterface $entity)protected method — unsets$this->patterns[$entityTypeId][$entityId][$langcode].updateEntityAlias()now callsresetEntityPatternCache()beforegetPatternByEntity(), ensuring conditions are re-evaluated against current entity values.Test coverage in
tests/src/Kernel/PathautoKernelTest.php:testEntityPatternCacheResetOnUpdate()kernel test./page/[node:title](weight -1) alongside the generic/content/[node:title](weight 10)./page/test-node, page pattern cached.updateEntityAlias()and asserts the alias remains/page/test-node— proving the stale cache was cleared and the correct bundle-specific pattern was re-evaluated./content/test-node).