Issue #1938960 by tim.plunkett, dawehner: Fixed _menu_translate() doesn't care about new routes.

core/includes/menu.inc

@@ -766,7 +766,18 @@ function _menu_translate(&$router_item, $map, $to_arg = FALSE) {
   $router_item['tab_root_href'] = implode('/', $tab_root_map);
   $router_item['tab_parent_href'] = implode('/', $tab_parent_map);
   $router_item['options'] = array();
-  _menu_check_access($router_item, $map);
+  if (!empty($router_item['route_name'])) {
+    $route_provider = Drupal::getContainer()->get('router.route_provider');
+    $route = $route_provider->getRouteByName($router_item['route_name']);
+    $request = Request::create('/' . $router_item['href']);
+    $request->attributes->set('system_path', $router_item['href']);
+    $request->attributes->add(Drupal::service('router')->matchRequest($request));
+    $router_item['access'] = Drupal::service('access_manager')->check($route, $request);
+  }
+  else {
+    // @todo: Remove once all routes are converted.
+    _menu_check_access($router_item, $map);
+  }
 
   // For performance, don't localize an item the user can't access.
   if ($router_item['access']) {
@@ -3166,6 +3177,7 @@ function _menu_router_build($callbacks, $save = FALSE) {
       'file' => '',
       'file path' => '',
       'include file' => '',
+      'route_name' => '',
     );
 
     // Calculate out the file to be included for each callback, if any.
@@ -3227,6 +3239,7 @@ function _menu_router_save($menu, $masks) {
       'position',
       'weight',
       'include_file',
+      'route_name',
     ));
 
   $num_records = 0;
@@ -3258,6 +3271,7 @@ function _menu_router_save($menu, $masks) {
       'position' => $item['position'],
       'weight' => $item['weight'],
       'include_file' => $item['include file'],
+      'route_name' => $item['route_name'],
     ));
 
     // Execute in batches to avoid the memory overhead of all of those records

core/lib/Drupal/Core/Access/DefaultAccessCheck.php

@@ -26,6 +26,6 @@ class DefaultAccessCheck implements AccessCheckInterface {
    * Implements AccessCheckInterface::access().
    */
   public function access(Route $route, Request $request) {
-    return (bool) $route->getRequirement('_access');
+    return $route->getRequirement('_access') === 'TRUE';
   }
 }

core/modules/system/lib/Drupal/system/Tests/Menu/MenuTranslateTest.php

@@ -0,0 +1,53 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Menu\MenuTranslateTest.
+ */
+
+namespace Drupal\system\Tests\Menu;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Defines a test class which tests the _menu_translate method.
+ *
+ * @see _menu_translate().
+ */
+class MenuTranslateTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('menu_test');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'menu_translate',
+      'description' => 'Tests the _menu_translate() method.',
+      'group' => 'Menu',
+    );
+  }
+
+  /**
+   * Tests _menu_translate().
+   */
+  public function testMenuTranslate() {
+    // Check for access to a restricted local task from a default local task.
+    $this->drupalGet('foo/asdf');
+    $this->assertResponse(200);
+    $this->assertLinkByHref('foo/asdf');
+    $this->assertLinkByHref('foo/asdf/b');
+    $this->assertNoLinkByHref('foo/asdf/c');
+
+    // Attempt to access a restricted local task.
+    $this->drupalGet('foo/asdf/c');
+    $this->assertResponse(403);
+    $this->assertNoLinkByHref('foo/asdf');
+    $this->assertNoLinkByHref('foo/asdf/b');
+    $this->assertNoLinkByHref('foo/asdf/c');
+  }
+
+}

core/modules/system/lib/Drupal/system/Tests/Menu/TreeAccessTest.php

@@ -67,7 +67,7 @@ class TreeAccessTest extends DrupalUnitTestBase {
           '_controller' => '\Drupal\menu_test\TestController::test'
         ),
         array(
-          '_access' => '1'
+          '_access' => 'TRUE'
         )
       ));
       $route_collection->add('menu_test_2', new Route('/menu_test/test_2',
@@ -75,7 +75,7 @@ class TreeAccessTest extends DrupalUnitTestBase {
           '_controller' => '\Drupal\menu_test\TestController::test'
         ),
         array(
-          '_access' => '0'
+          '_access' => 'FALSE'
         )
       ));
       $this->routeCollection = $route_collection;

core/modules/system/system.install

@@ -858,6 +858,11 @@ function system_schema() {
         'type' => 'text',
         'size' => 'medium',
       ),
+      'route_name' => array(
+        'description' => 'The machine name of a defined Symfony Route this menu item represents.',
+        'type' => 'varchar',
+        'length' => 255,
+      ),
     ),
     'indexes' => array(
       'fit' => array('fit'),
@@ -2082,6 +2087,19 @@ function system_update_8053() {
   db_drop_table('cache_form');
 }
 
+/**
+ * Adds route_name column to the menu_router table.
+ */
+function system_update_8054() {
+  $spec = array(
+    'description' => 'The machine name of a defined Symfony Route this menu item represents.',
+    'type' => 'varchar',
+    'length' => 255,
+  );
+
+  db_add_field('menu_router', 'route_name', $spec);
+}
+
 /**
  * @} End of "defgroup updates-7.x-to-8.x".
  * The next series of updates should start at 9000.

core/modules/system/tests/modules/menu_test/menu_test.module

@@ -407,6 +407,11 @@ function menu_test_menu() {
   // Controller-based local task.
   $items['foo/%/b'] = array(
     'title' => 'Local task B',
+    'route_name' => 'menu_router_test2',
+    'type' => MENU_LOCAL_TASK,
+  );
+  $items['foo/%/c'] = array(
+    'title' => 'Local task C',
     'route_name' => 'menu_router_test3',
     'type' => MENU_LOCAL_TASK,
   );

core/modules/system/tests/modules/menu_test/menu_test.routing.yml

@@ -4,9 +4,15 @@ menu_router_test1:
     _content: '\Drupal\menu_test\TestControllers::test1'
   requirements:
     _access: 'TRUE'
-menu_router_test3:
+menu_router_test2:
   pattern: '/foo/{bar}/b'
   defaults:
     _content: '\Drupal\menu_test\TestControllers::test2'
   requirements:
     _access: 'TRUE'
+menu_router_test3:
+  pattern: '/foo/{bar}/c'
+  defaults:
+    _content: '\Drupal\menu_test\TestControllers::test2'
+  requirements:
+    _access: 'FALSE'

core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php

@@ -0,0 +1,75 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Access\DefaultAccessCheckTest.
+ */
+
+namespace Drupal\Tests\Core\Access;
+
+use Drupal\Core\Access\DefaultAccessCheck;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Route;
+
+/**
+ * Defines a test to check the default access checker.
+ *
+ * @see \Drupal\Core\Access\DefaultAccessCheck
+ */
+class DefaultAccessCheckTest extends UnitTestCase {
+
+  /**
+   * The access checker to test.
+   *
+   * @var \Drupal\Core\Access\DefaultAccessCheck
+   */
+  protected $accessChecker;
+
+  public static function getInfo() {
+    return array(
+      'name' => 'DefaultAccessCheck access checker',
+      'description' => 'Tests the DefaultAccessCheck class.',
+      'group' => 'Routing',
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->accessChecker = new DefaultAccessCheck();
+  }
+
+
+  /**
+   * Test the applies method.
+   */
+  public function testApplies() {
+    $route = new Route('/test-route');
+    $this->assertFalse($this->accessChecker->applies($route), 'Access checker applied even no _access was defined as requirement.');
+
+    $route->addRequirements(array('_access' => 'FALSE'));
+    $this->assertTrue($this->accessChecker->applies($route), 'Access checker applied even a _access was defined as requirement.');
+
+    $route->addRequirements(array('_access' => 'TRUE'));
+    $this->assertTrue($this->accessChecker->applies($route), 'Access checker applied even a _access was defined as requirement.');
+  }
+
+  /**
+   * Test the access method.
+   */
+  public function testAccess() {
+    $route = new Route('/test-route');
+    $request = new Request(array());
+
+    $route->addRequirements(array('_access' => 'FALSE'));
+    $this->assertFalse($this->accessChecker->access($route, $request));
+
+    $route->addRequirements(array('_access' => 'TRUE'));
+    $this->assertTrue($this->accessChecker->access($route, $request));
+  }
+
+}