If you implement hook_pattern_alter(), and change the pattern, then because the pattern is static cached, your altered pattern will take affect even when you don't want it to. This won't be a problem if a single path is being generated, but if multiple paths are being generated in a single HTTP request (e.g. bulk (re)generation, or migrations), then the (altered) statically cached pattern will always take affect, even when the conditions within your alter hook are not met.

Example implementation where this bug happens:

/**
 * Implements hook_pathauto_pattern_alter().
 */
function my_module_pathauto_pattern_alter(\Drupal\pathauto\PathautoPatternInterface &$pattern, array $context) {
  /* @var $node Drupal\node\Entity\Node */
  if ($context['module'] !== 'node' || !($node = $context['data']['node']) || $node->bundle() !== 'resource') {
    return;
  }

  if ($node->field_issues) {
    // Our new pattern is almost the same, but without the leading "education/".
    $pattern->setPattern(preg_replace('!^education/!u', '', $pattern->getPattern()));
  }
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dalin created an issue. See original summary.

dalin’s picture

Assigned: dalin » Unassigned
Status: Active » Needs review
FileSize
1.17 KB
Berdir’s picture

Status: Needs review » Fixed

Yeah, that's exactly why the hook like this is problematic. But it's there, the module is stable and we documented it now like this, so we have to live with it.

Thought about a test, but not trivial to set up I suppose and at least we it doesn't break anything we have tests for.

  • Berdir committed 37d3b42 on 8.x-1.x authored by dalin
    Issue #2950643 by dalin: Implementing hook_pathauto_pattern_alter()...
Berdir’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.