Fixed 'convertPermissions()' against TranslatableMarkup titles and restored 'drupal_static()' cache. (#333)

Author: AlexSkrypnyk

rector.php

@@ -13,6 +13,7 @@
 use Rector\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector;
 use Rector\CodingStyle\Rector\Stmt\NewlineAfterStatementRector;
 use Rector\Config\RectorConfig;
+use Rector\EarlyReturn\Rector\StmtsAwareInterface\ReturnEarlyIfVariableRector;
 use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector;
 
 return RectorConfig::configure()
@@ -36,6 +37,11 @@
         CatchExceptionNameMatchingTypeRector::class,
         // Too aggressive for mixed-type codebase.
         DisallowedEmptyRuleFixerRector::class,
+        // Breaks 'drupal_static()' caching in 'Drupal8::getAllPermissions()':
+        // assigning into the static reference is required for subsequent
+        // calls to hit the cache, so the intermediate variable is not dead
+        // code even though Rector thinks it is.
+        ReturnEarlyIfVariableRector::class,
         // Legacy test with PHPUnit compatibility issue.
         __DIR__ . '/tests/Drupal/Tests/Driver/Drupal7FieldHandlerTest.php',
         // Dependencies.

src/Drupal/Driver/Cores/Drupal8.php

@@ -209,7 +209,7 @@ protected function getAllPermissions() {
     $permissions = &drupal_static(__FUNCTION__);
 
     if (!isset($permissions)) {
-      return \Drupal::service('user.permissions')->getPermissions();
+      $permissions = \Drupal::service('user.permissions')->getPermissions();
     }
 
     return $permissions;
@@ -225,8 +225,11 @@ protected function convertPermissions(array &$permissions) {
     $all_permissions = $this->getAllPermissions();
 
     foreach ($all_permissions as $name => $definition) {
-      $key = array_search($definition['title'], $permissions, TRUE);
-      if (FALSE !== $key) {
+      // Cast the title to string: Drupal returns TranslatableMarkup objects
+      // for permission titles, which would never strictly equal the plain
+      // string labels supplied by callers.
+      $key = array_search((string) $definition['title'], $permissions, TRUE);
+      if ($key !== FALSE) {
         $permissions[$key] = $name;
       }
     }

tests/Drupal/Tests/Driver/Drupal8PermissionsTest.php

@@ -0,0 +1,153 @@
+<?php
+
+namespace Drupal\Tests\Driver;
+
+use Drupal\Driver\Cores\Drupal8;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Tests permission label and machine name conversion in the Drupal 8+ driver.
+ */
+class Drupal8PermissionsTest extends TestCase {
+
+  /**
+   * Tests that human-readable titles are converted to machine names.
+   *
+   * Drupal returns permission titles as TranslatableMarkup objects. Strict
+   * comparison against plain string labels would never match, so the driver
+   * must cast the title to string before lookup. This guards against
+   * regressions where automated refactoring flips the comparison to strict
+   * mode without adding the cast.
+   */
+  public function testConvertPermissionsMapsStringableTitlesToMachineNames() {
+    $core = new TestDrupal8PermissionsCore(__DIR__, 'default');
+    $core->setPermissions([
+      'administer content types' => [
+        'title' => $this->stringable('Administer content types'),
+      ],
+      'administer users' => [
+        'title' => $this->stringable('Administer users'),
+      ],
+    ]);
+
+    $permissions = ['Administer content types', 'Administer users'];
+    $this->callConvertPermissions($core, $permissions);
+
+    $this->assertSame(['administer content types', 'administer users'], $permissions);
+  }
+
+  /**
+   * Tests that titles already in machine name form are left unchanged.
+   */
+  public function testConvertPermissionsLeavesMachineNamesAlone() {
+    $core = new TestDrupal8PermissionsCore(__DIR__, 'default');
+    $core->setPermissions([
+      'administer users' => [
+        'title' => $this->stringable('Administer users'),
+      ],
+    ]);
+
+    $permissions = ['administer users'];
+    $this->callConvertPermissions($core, $permissions);
+
+    $this->assertSame(['administer users'], $permissions);
+  }
+
+  /**
+   * Tests that 'checkPermissions()' passes for valid machine names.
+   */
+  public function testCheckPermissionsAcceptsValidMachineNames() {
+    $core = new TestDrupal8PermissionsCore(__DIR__, 'default');
+    $core->setPermissions([
+      'administer users' => ['title' => 'Administer users'],
+      'access content' => ['title' => 'Access content'],
+    ]);
+
+    $permissions = ['administer users', 'access content'];
+    $this->callCheckPermissions($core, $permissions);
+
+    $this->assertSame(['administer users', 'access content'], $permissions);
+  }
+
+  /**
+   * Tests that 'checkPermissions()' throws for unknown machine names.
+   */
+  public function testCheckPermissionsThrowsForUnknownPermission() {
+    $core = new TestDrupal8PermissionsCore(__DIR__, 'default');
+    $core->setPermissions([
+      'administer users' => ['title' => 'Administer users'],
+    ]);
+
+    $permissions = ['administer users', 'unknown permission'];
+
+    $this->expectException(\RuntimeException::class);
+    $this->expectExceptionMessage('Invalid permission "unknown permission".');
+
+    $this->callCheckPermissions($core, $permissions);
+  }
+
+  /**
+   * Invokes the protected 'convertPermissions()' method by reference.
+   */
+  protected function callConvertPermissions(Drupal8 $core, array &$permissions) {
+    $method = new \ReflectionMethod($core, 'convertPermissions');
+    $method->setAccessible(TRUE);
+    $method->invokeArgs($core, [&$permissions]);
+  }
+
+  /**
+   * Invokes the protected 'checkPermissions()' method by reference.
+   */
+  protected function callCheckPermissions(Drupal8 $core, array &$permissions) {
+    $method = new \ReflectionMethod($core, 'checkPermissions');
+    $method->setAccessible(TRUE);
+    $method->invokeArgs($core, [&$permissions]);
+  }
+
+  /**
+   * Returns an anonymous Stringable that mimics a Drupal TranslatableMarkup.
+   */
+  protected function stringable($label) {
+    return new class($label) {
+
+      public function __construct(private $label) {}
+
+      /**
+       * Renders the stringable into its label.
+       */
+      public function __toString(): string {
+        return $this->label;
+      }
+
+    };
+  }
+
+}
+
+/**
+ * Testable subclass that overrides 'getAllPermissions()'.
+ */
+class TestDrupal8PermissionsCore extends Drupal8 {
+
+  /**
+   * Stored permissions keyed by machine name.
+   *
+   * @var array
+   */
+  protected $permissions = [];
+
+  /**
+   * Sets the permissions returned by 'getAllPermissions()'.
+   */
+  public function setPermissions(array $permissions) {
+    $this->permissions = $permissions;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function getAllPermissions() {
+    return $this->permissions;
+  }
+
+}