From 35a08a6780f7113d793e6285fdd62edd7bdec602 Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Tue, 30 Nov 2010 01:05:24 +0000 Subject: [PATCH] - Patch #863318 by carlos8f, andypost, chx, Dave Reid, plach: wrong sort order of aliases for different languages. --- includes/path.inc | 64 ++++++++++++++++----- modules/locale/locale.test | 53 +++++++++++++++++- modules/simpletest/tests/path.test | 90 ++++++++++++++++++++++++++++++ modules/system/system.install | 16 +++++- 4 files changed, 208 insertions(+), 15 deletions(-) diff --git a/includes/path.inc b/includes/path.inc index 3b5549656786..83ebcb0c972c 100644 --- a/includes/path.inc +++ b/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 { diff --git a/modules/locale/locale.test b/modules/locale/locale.test index cf23d6b048d4..d103adedc37a 100644 --- a/modules/locale/locale.test +++ b/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.')); } } diff --git a/modules/simpletest/tests/path.test b/modules/simpletest/tests/path.test index f995856912a7..7368cd1d4047 100644 --- a/modules/simpletest/tests/path.test +++ b/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.')); + } +} diff --git a/modules/system/system.install b/modules/system/system.install index 8a255ba82557..01924300a3b9 100644 --- a/modules/system/system.install +++ b/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. -- GitLab