- Patch #863318 by carlos8f, andypost, chx, Dave Reid, plach: wrong sort order...

- Patch #863318 by carlos8f, andypost, chx, Dave Reid, plach: wrong sort order of aliases for different languages.

includes/path.inc

@@ -94,13 +94,30 @@ function drupal_lookup_path($action, $path = '', $path_language = NULL) {
         if ($cached = cache_get($cid, 'cache_path')) {
           $cache['system_paths'] = $cached->data;
           // Now fetch the aliases corresponding to these system paths.
-          // We order by ASC and overwrite array keys to ensure the correct
-          // alias is used when there are multiple aliases per path.
-          $cache['map'][$path_language] = db_query("SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND language IN (:language, :language_none) ORDER BY language ASC, pid ASC", array(
+          $args = array(
             ':system' => $cache['system_paths'],
             ':language' => $path_language,
             ':language_none' => LANGUAGE_NONE,
-          ))->fetchAllKeyed();
+          );
+          // Always get the language-specific alias before the language-neutral
+          // one. For example 'de' is less than 'und' so the order needs to be
+          // ASC, while 'xx-lolspeak' is more than 'und' so the order needs to
+          // be DESC. We also order by pid ASC so that fetchAllKeyed() returns
+          // the most recently created alias for each source. Subsequent queries
+          // using fetchField() must use pid DESC to have the same effect.
+          // For performance reasons, the query builder is not used here.
+          if ($path_language == LANGUAGE_NONE) {
+            // Prevent PDO from complaining about a token the query doesn't use.
+            unset($args[':language']);
+            $result = db_query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND language = :language_none ORDER BY pid ASC', $args);
+          }
+          elseif ($path_language < LANGUAGE_NONE) {
+            $result = db_query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND language IN (:language, :language_none) ORDER BY language ASC, pid ASC', $args);
+          }
+          else {
+            $result = db_query('SELECT source, alias FROM {url_alias} WHERE source IN (:system) AND language IN (:language, :language_none) ORDER BY language DESC, pid ASC', $args);
+          }
+          $cache['map'][$path_language] = $result->fetchAllKeyed();
           // Keep a record of paths with no alias to avoid querying twice.
           $cache['no_aliases'][$path_language] = array_flip(array_diff_key($cache['system_paths'], array_keys($cache['map'][$path_language])));
         }
@@ -117,12 +134,22 @@ function drupal_lookup_path($action, $path = '', $path_language = NULL) {
       }
       // For system paths which were not cached, query aliases individually.
       elseif (!isset($cache['no_aliases'][$path_language][$path])) {
-        // Get the most fitting result falling back with alias without language
-        $alias = db_query("SELECT alias FROM {url_alias} WHERE source = :source AND language IN (:language, :language_none) ORDER BY language DESC, pid DESC", array(
+        $args = array(
           ':source' => $path,
           ':language' => $path_language,
           ':language_none' => LANGUAGE_NONE,
-        ))->fetchField();
+        );
+        // See the queries above.
+        if ($path_language == LANGUAGE_NONE) {
+          unset($args[':language']);
+          $alias = db_query("SELECT alias FROM {url_alias} WHERE source = :source AND language = :language_none ORDER BY pid DESC", $args)->fetchField();
+        }
+        elseif ($path_language > LANGUAGE_NONE) {
+          $alias = db_query("SELECT alias FROM {url_alias} WHERE source = :source AND language IN (:language, :language_none) ORDER BY language DESC, pid DESC", $args)->fetchField();
+        }
+        else {
+          $alias = db_query("SELECT alias FROM {url_alias} WHERE source = :source AND language IN (:language, :language_none) ORDER BY language ASC, pid DESC", $args)->fetchField();
+        }
         $cache['map'][$path_language][$path] = $alias;
         return $alias;
       }
@@ -133,12 +160,23 @@ function drupal_lookup_path($action, $path = '', $path_language = NULL) {
       // Look for the value $path within the cached $map
       $source = FALSE;
       if (!isset($cache['map'][$path_language]) || !($source = array_search($path, $cache['map'][$path_language]))) {
-        // Get the most fitting result falling back with alias without language
-        if ($source = db_query("SELECT source FROM {url_alias} WHERE alias = :alias AND language IN (:language, :language_none) ORDER BY language DESC, pid DESC", array(
-                     ':alias' => $path,
-                     ':language' => $path_language,
-                     ':language_none' => LANGUAGE_NONE))
-            ->fetchField()) {
+        $args = array(
+          ':alias' => $path,
+          ':language' => $path_language,
+          ':language_none' => LANGUAGE_NONE,
+        );
+        // See the queries above.
+        if ($path_language == LANGUAGE_NONE) {
+          unset($args[':language']);
+          $result = db_query("SELECT source FROM {url_alias} WHERE alias = :alias AND language = :language_none ORDER BY pid DESC", $args);
+        }
+        elseif ($path_language > LANGUAGE_NONE) {
+          $result = db_query("SELECT source FROM {url_alias} WHERE alias = :alias AND language IN (:language, :language_none) ORDER BY language DESC, pid DESC", $args);
+        }
+        else {
+          $result = db_query("SELECT source FROM {url_alias} WHERE alias = :alias AND language IN (:language, :language_none) ORDER BY language ASC, pid DESC", $args);
+        }
+        if ($source = $result->fetchField()) {
           $cache['map'][$path_language][$source] = $path;
         }
         else {

modules/locale/locale.test

@@ -1577,7 +1577,58 @@ class LocalePathFunctionalTest extends DrupalWebTestCase {
     $this->drupalGet($prefix . '/' . $custom_language_path);
     $this->assertText($node->title, t('Custom language alias works.'));
 
-    $this->drupalLogout();
+    // Create a custom path.
+    $custom_path = $this->randomName(8);
+
+    // Check priority of language for alias by source path.
+    $edit = array(
+      'source'   => 'node/' . $node->nid,
+      'alias'    => $custom_path,
+      'language' => LANGUAGE_NONE,
+    );
+    path_save($edit);
+    $lookup_path = drupal_lookup_path('alias', 'node/' . $node->nid, 'en');
+    $this->assertEqual($english_path, $lookup_path, t('English language alias has priority.'));
+    // Same check for language 'xx'.
+    $lookup_path = drupal_lookup_path('alias', 'node/' . $node->nid, $prefix);
+    $this->assertEqual($custom_language_path, $lookup_path, t('Custom language alias has priority.'));
+    path_delete($edit);
+
+    // Create language nodes to check priority of aliases.
+    $first_node = $this->drupalCreateNode(array('type' => 'article', 'promote' => 1));
+    $second_node = $this->drupalCreateNode(array('type' => 'article', 'promote' => 1));
+
+    // Assign a custom path alias to the first node with the English language.
+    $edit = array(
+      'source'   => 'node/' . $first_node->nid,
+      'alias'    => $custom_path,
+      'language' => 'en',
+    );
+    path_save($edit);
+
+    // Assign a custom path alias to second node with LANGUAGE_NONE.
+    $edit = array(
+      'source'   => 'node/' . $second_node->nid,
+      'alias'    => $custom_path,
+      'language' => LANGUAGE_NONE,
+    );
+    path_save($edit);
+
+    // Test that both node titles link to our path alias.
+    $this->drupalGet('<front>');
+    $custom_path_url = base_path() . (variable_get('clean_url', 0) ? $custom_path : '?q=' . $custom_path);
+    $elements = $this->xpath('//a[@href=:href and .=:title]', array(':href' => $custom_path_url, ':title' => $first_node->title));
+    $this->assertTrue(!empty($elements), t('First node links to the path alias.'));
+    $elements = $this->xpath('//a[@href=:href and .=:title]', array(':href' => $custom_path_url, ':title' => $second_node->title));
+    $this->assertTrue(!empty($elements), t('Second node links to the path alias.'));
+
+    // Confirm that the custom path leads to the first node.
+    $this->drupalGet($custom_path);
+    $this->assertText($first_node->title, t('Custom alias returns first node.'));
+
+    // Confirm that the custom path with prefix leads to the second node.
+    $this->drupalGet($prefix . '/' . $custom_path);
+    $this->assertText($second_node->title, t('Custom alias with prefix returns second node.'));
   }
 }
 

modules/simpletest/tests/path.test

@@ -236,3 +236,93 @@ class UrlAlterFunctionalTest extends DrupalWebTestCase {
     $this->assertIdentical($result, $final, t('Altered inbound URL %original, expected %final, and got %result.', array('%original' => $original, '%final' => $final, '%result' => $result)));
   }
 }
+
+/**
+ * Unit test for drupal_lookup_path().
+ */
+class PathLookupTest extends DrupalWebTestCase {
+  public static function getInfo() {
+    return array(
+      'name' => t('Path lookup'),
+      'description' => t('Tests that drupal_lookup_path() returns correct paths.'),
+      'group' => t('Path API'),
+    );
+  }
+
+  /**
+   * Test that drupal_lookup_path() returns the correct path.
+   */
+  function testDrupalLookupPath() {
+    $account = $this->drupalCreateUser();
+    $uid = $account->uid;
+    $name = $account->name;
+
+    // Test the situation where the source is the same for multiple aliases.
+    // Start with a language-neutral alias, which we will override.
+    $path = array(
+      'source' => "user/$uid",
+      'alias' => 'foo',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), $path['alias'], t('Basic alias lookup works.'));
+    $this->assertEqual(drupal_lookup_path('source', $path['alias']), $path['source'], t('Basic source lookup works.'));
+
+    // Create a language specific alias for the default language (English).
+    $path = array(
+      'source' => "user/$uid",
+      'alias' => "users/$name",
+      'language' => 'en',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), $path['alias'], t('English alias overrides language-neutral alias.'));
+    $this->assertEqual(drupal_lookup_path('source', $path['alias']), $path['source'], t('English source overrides language-neutral source.'));
+
+    // Create a language-neutral alias for the same path, again.
+    $path = array(
+      'source' => "user/$uid",
+      'alias' => 'bar',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), "users/$name", t('English alias still returned after entering a language-neutral alias.'));
+
+    // Create a language-specific (xx-lolspeak) alias for the same path.
+    $path = array(
+      'source' => "user/$uid",
+      'alias' => 'LOL',
+      'language' => 'xx-lolspeak',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), "users/$name", t('English alias still returned after entering a LOLspeak alias.'));
+    // The LOLspeak alias should be returned if we really want LOLspeak.
+    $this->assertEqual(drupal_lookup_path('alias', $path['source'], 'xx-lolspeak'), 'LOL', t('LOLspeak alias returned if we specify xx-lolspeak to drupal_lookup_path().'));
+
+    // Create a new alias for this path in English, which should override the
+    // previous alias for "user/$uid".
+    $path = array(
+      'source' => "user/$uid",
+      'alias' => 'users/my-new-path',
+      'language' => 'en',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), $path['alias'], t('Recently created English alias returned.'));
+    $this->assertEqual(drupal_lookup_path('source', $path['alias']), $path['source'], t('Recently created English source returned.'));
+
+    // Remove the English aliases, which should cause a fallback to the most
+    // recently created language-neutral alias, 'bar'.
+    db_delete('url_alias')
+      ->condition('language', 'en')
+      ->execute();
+    drupal_clear_path_cache();
+    $this->assertEqual(drupal_lookup_path('alias', $path['source']), 'bar', t('Path lookup falls back to recently created language-neutral alias.'));
+
+    // Test the situation where the alias and language are the same, but
+    // the source differs. The newer alias record should be returned.
+    $account2 = $this->drupalCreateUser();
+    $path = array(
+      'source' => 'user/' . $account2->uid,
+      'alias' => 'bar',
+    );
+    path_save($path);
+    $this->assertEqual(drupal_lookup_path('source', $path['alias']), $path['source'], t('Newer alias record is returned when comparing two LANGUAGE_NONE paths with the same alias.'));
+  }
+}

modules/system/system.install

@@ -1630,7 +1630,7 @@ function system_schema() {
         'default' => '',
       ),
       'language' => array(
-        'description' => 'The language this alias is for; if blank, the alias will be used for unknown languages. Each Drupal path can have an alias for each supported language.',
+        'description' => "The language this alias is for; if 'und', the alias will be used for unknown languages. Each Drupal path can have an alias for each supported language.",
         'type' => 'varchar',
         'length' => 12,
         'not null' => TRUE,
@@ -2950,6 +2950,20 @@ function system_update_7067() {
   user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array('view the administration theme'));
 }
 
+/**
+ * Update {url_alias}.language description.
+ */
+function system_update_7068() {
+  $spec = array(
+    'description' => "The language this alias is for; if 'und', the alias will be used for unknown languages. Each Drupal path can have an alias for each supported language.",
+    'type' => 'varchar',
+    'length' => 12,
+    'not null' => TRUE,
+    'default' => '',
+  );
+  db_change_field('url_alias', 'language', 'language', $spec);
+}
+
 /**
  * @} End of "defgroup updates-6.x-to-7.x"
  * The next series of updates should start at 8000.