Issue backdrop#905: Further cleanup for Redirect. Merging https://www…

….drupal.org/node/1025904 to fix hook_path_delete().

core/includes/path.inc

@@ -460,14 +460,12 @@ function path_delete($criteria) {
   if (!is_array($criteria)) {
     $criteria = array('pid' => $criteria);
   }
-  $path = path_load($criteria);
-  $query = db_delete('url_alias');
-  foreach ($criteria as $field => $value) {
-    $query->condition($field, $value);
+  // Load and then delete each path until there are no more aliases left.
+  while ($path = path_load($criteria)) {
+    module_invoke_all('path_delete', $path);
+    backdrop_clear_path_cache($path['source']);
+    db_delete('url_alias')->condition('pid', $path['pid'])->execute();
   }
-  $query->execute();
-  module_invoke_all('path_delete', $path);
-  backdrop_clear_path_cache($path['source']);
 }
 
 /**

core/modules/path/tests/path.test

@@ -518,3 +518,60 @@ class PathMonolingualTestCase extends BackdropWebTestCase {
     $this->assertText(t('Add language'), 'Page contains the add language text');
   }
 }
+
+/**
+ * Tests that path hooks are invoked.
+ */
+class PathHooksTestCase extends BackdropWebTestCase {
+  protected $profile = 'testing';
+
+  function setUp() {
+    parent::setUp(array('node', 'path', 'path_test'));
+    $this->backdropCreateContentType(array(
+      'type' => 'page',
+      'name' => 'Page',
+    ));
+    $web_user = $this->backdropCreateUser(array('access content', 'create page content', 'delete own page content', 'administer url aliases', 'create url aliases'));
+    $this->backdropLogin($web_user);
+  }
+
+  function testPathHooks() {
+    // Create test node.
+    $node1 = $this->backdropCreateNode();
+
+    // Generate two test aliases.
+    $alias1 = $this->randomName(8);
+    $alias2 = $this->randomName(8);
+
+    // Insert aliases and test that hook_path_insert() is called.
+    $edit = array();
+    $edit['source'] = 'node/' . $node1->nid;
+    $edit['alias'] = $alias1;
+    $this->backdropPost('admin/config/search/path/add', $edit, t('Save URL alias'));
+    $this->assertRaw('path_test_path_insert(): ' . $edit['alias'] . ' => ' . $edit['source'], t('hook_path_insert() was called.'));
+
+    $edit['alias'] = $alias2;
+    $this->backdropPost('admin/config/search/path/add', $edit, t('Save URL alias'));
+
+    // Extract the path ID from the second path alias.
+    $inserted_path = state_get('path_test_inserted_path', array());
+    $pid2 = $inserted_path['pid'];
+
+    // Update the second path alias and test that hook_path_update() is called.
+    $edit['alias'] = $alias2 = $this->randomName(8);
+    $this->backdropPost('admin/config/search/path/edit/' . $pid2, $edit, t('Save URL alias'));
+    $this->assertRaw('path_test_path_update(): ' . $edit['alias'] . ' => ' . $edit['source'], t('hook_path_update() was called.'));
+
+    // Delete the node and test that hook_path_delete() is called once for each
+    // path alias.
+    $this->backdropPost('node/' . $node1->nid . '/delete', array(), t('Delete'));
+    $this->assertRaw('path_test_path_delete(): ' . $alias1 . ' => node/' . $node1->nid, t('hook_path_delete() was called for the first path alias.'));
+    $this->assertRaw('path_test_path_delete(): ' . $alias2 . ' => node/' . $node1->nid, t('hook_path_delete() was called for the second path alias.'));
+
+    // Test that hook_path_delete() is not called if the node has no path
+    // aliases.
+    $node2 = $this->backdropCreateNode();
+    $this->backdropPost('node/' . $node2->nid . '/delete', array(), t('Delete'));
+    $this->assertNoRaw('path_test_path_delete()', t('hook_path_delete() was not called.'));
+  }
+}

core/modules/path/tests/path.tests.info

@@ -28,6 +28,12 @@ description = Confirm that paths are not changed on monolingual non-English site
 group = Path
 file = path.test
 
+[PathHooksTestCase]
+name = Path hooks
+description = Test the hooks invoked when a path is inserted, updated, or deleted.
+group = Path
+file = path.test
+
 [PathPatternUnitTestCase]
 name = Path pattern unit tests
 description = Unit tests for Path automatic alias functions.

core/modules/redirect/redirect.admin.inc

@@ -78,7 +78,7 @@ function redirect_list_form($form, &$form_state) {
   $rows = array();
   foreach ($redirects as $rid => $redirect) {
     $row = array();
-    $redirect->source_options = array_merge($redirect->source_options, array('alias' => TRUE, 'langcode' => redirect_language_load($redirect->langcode)));
+    $redirect->source_options = array_merge($redirect->source_options, array('alias' => TRUE));
     $source_url = redirect_url($redirect->source, $redirect->source_options);
     $redirect_url = redirect_url($redirect->redirect, array_merge($redirect->redirect_options, array('alias' => TRUE)));
     backdrop_alter('redirect_url', $redirect->source, $redirect->source_options);
@@ -861,7 +861,7 @@ function redirect_list_table($redirects, $header) {
   $rows = array();
   foreach ($redirects as $rid => $redirect) {
     $row = array();
-    $redirect->source_options = array_merge($redirect->source_options, array('alias' => TRUE, 'language' => redirect_language_load($redirect->langcode)));
+    $redirect->source_options = array_merge($redirect->source_options, array('alias' => TRUE, 'language' => language_load($redirect->langcode)));
     $source_url = redirect_url($redirect->source, $redirect->source_options);
     $redirect_url = redirect_url($redirect->redirect, array_merge($redirect->redirect_options, array('alias' => TRUE)));
     $row['data']['source'] = l($source_url, $redirect->source, $redirect->source_options);

core/modules/redirect/redirect.module

@@ -342,18 +342,13 @@ function redirect_path_update(array $path) {
 /**
  * Implements hook_path_delete().
  */
-function redirect_path_delete($path) {
+function redirect_path_delete(array $path) {
   if (!config_get('redirect.settings', 'auto_redirect')) {
     return;
   }
   elseif (isset($path['redirect']) && !$path['redirect']) {
     return;
   }
-  elseif (empty($path)) {
-    // @todo Remove this condition and allow $path to use an array type hint
-    // when http://drupal.org/node/1025904 is fixed.
-    return;
-  }
 
   // Redirect from a deleted alias to the system path.
   if (!redirect_load_by_source($path['alias'], $path['langcode'])) {
@@ -661,8 +656,8 @@ function redirect_validate(Redirect $redirect, $form, &$form_state) {
 /**
  * Save an URL redirect.
  *
- * @param $redirect
- *   The URL redirect object to be saved. If $redirect->rid is omitted (or
+ * @param Redirect $redirect
+ *   The Redirect object to be saved. If $redirect->rid is omitted (or
  *   $redirect->is_new is TRUE), a new redirect will be added.
  *
  * @ingroup redirect_api
@@ -710,7 +705,7 @@ function redirect_save(Redirect $redirect) {
     redirect_load_multiple(array(), array(), TRUE);
 
     // Ignore slave server temporarily to give time for the
-    // saved node to be propagated to the slave.
+    // saved redirect to be propagated to the slave.
     db_ignore_slave();
   }
   catch (Exception $e) {
@@ -976,42 +971,6 @@ function redirect_page_cache_clear(Redirect $redirect) {
   cache('page')->deletePrefix($path);
 }
 
-// TODO: @gff commented this out per https://github.com/backdrop/backdrop/pull/1249#discussion_r52810547
-// if redirect module is still working without redirect_load_entity_from_path()
-// and the above function can not be fixed, let's remove it from the codebase.
-/**
- * Given a path determine if it is an entity default path.
- *
- * @param $path
- *   The internal path. The id of the entity should be in the string as '[id]'.
- * @return
- *   An array with the entity type and the loaded entity object.
- */
-// function redirect_load_entity_from_path($path) {
-//   $entity_paths = &backdrop_static(__FUNCTION__);
-//
-//   if (!isset($entity_paths)) {
-//     $entity_paths = array();
-//     foreach (entity_get_info() as $entity_type => $entity_info) {
-//       if (isset($entity_info['default path'])) {
-//         $default_path = $entity_info['default path'];
-//         $default_path = preg_quote($default_path, '/');
-//         $default_path = str_replace(preg_quote('%' . $entity_type, '/'), '(\d+)', $default_path);
-//         $entity_paths[$entity_type] = $default_path;
-//       }
-//     }
-//   }
-//
-//   foreach ($entity_paths as $entity_type => $default_path) {
-//     if (preg_match("/^{$default_path}$/", $path, $matches)) {
-//       if ($entity = entity_load($entity_type, array($matches[1]))) {
-//         return array('entity_type' => $entity_type, 'entity' => reset($entity));
-//       }
-//       break;
-//     }
-//   }
-// }
-
 /**
  * Check the ability to perform redirects with the current request context.
  *
@@ -1106,28 +1065,6 @@ function redirect_sort_recursive(&$array, $callback = 'sort') {
   return $result;
 }
 
-/**
- * Load a language object by its language code.
- * @gff take this function out and replace calls with language_load().
- *
- * @todo Remove when http://drupal.org/node/660736 is fixed in Drupal core.
- *
- * @param $langcode
- *   A language code. If not provided the default language will be returned.
- * @return
- *   A language object.
- */
-function redirect_language_load($langcode = LANGUAGE_NONE) {
-  $languages = &backdrop_static(__FUNCTION__);
-
-  if (!isset($languages)) {
-    $languages = language_list();
-    $languages[LANGUAGE_NONE] = NULL;
-  }
-
-  return isset($languages[$langcode]) ? $languages[$langcode] : NULL;
-}
-
 /**
  * Build the URL of a redirect for display purposes only.
  */

core/modules/simpletest/tests/path_test.module

@@ -1,5 +1,4 @@
 <?php
-
 /**
  * @file
  * Helper module for the path tests.
@@ -12,10 +11,31 @@ function path_test_reset() {
   state_set('path_test_results', array());
 }
 
+/**
+ * Implements hook_path_delete().
+ */
+function path_test_path_delete($path) {
+  backdrop_set_message('path_test_path_delete(): ' . $path['alias'] . ' => ' . $path['source']);
+}
+
+/**
+ * Implements hook_path_insert().
+ */
+function path_test_path_insert($path) {
+  backdrop_set_message('path_test_path_insert(): ' . $path['alias'] . ' => ' . $path['source']);
+
+  // Save the path to a variable so the test routine can use the path ID.
+  // See PathHooksTestCase::testPathHooks().
+  state_set('path_test_inserted_path', $path);
+}
+
 /**
  * Implements hook_path_update().
  */
 function path_test_path_update($path) {
+  backdrop_set_message('path_test_path_update(): ' . $path['alias'] . ' => ' . $path['source']);
+
+  // @see PathSaveTest::testBackdropSaveOriginalPath().
   $results = state_get('path_test_results', array());
   $results['hook_path_update'] = $path;
   state_set('path_test_results', $results);