#496516 by Crell and Berdir: Moved query_alter() into a preExecute() method, so that modules can know the final query/arguments before they are run.

4 files changed, 123 insertions, 7 deletions

diff --git a/includes/database/query.inc b/includes/database/query.inc
index b6b47f48d..729b8a739 100644
--- a/includes/database/query.inc
+++ b/includes/database/query.inc
@@ -433,6 +433,8 @@ class InsertQuery extends Query {
    *   in multi-insert loops.
    */
   public function execute() {
+    // If validation fails, simply return NULL.
+    // Note that validation routines in preExecute() may throw exceptions instead.
     if (!$this->preExecute()) {
       return NULL;
     }
@@ -489,7 +491,7 @@ class InsertQuery extends Query {
    * @return
    *   TRUE if the validation was successful, FALSE if not.
    */
-  protected function preExecute() {
+  public function preExecute() {
     // Confirm that the user did not try to specify an identical
     // field and default field.
     if (array_intersect($this->insertFields, $this->defaultFields)) {
@@ -721,13 +723,29 @@ class MergeQuery extends Query {
     return $this;
   }
 
-  public function execute() {
+  /**
+   * Generic preparation and validation for a MERGE query.
+   *
+   * @return
+   *   TRUE if the validation was successful, FALSE if not.
+   */
+  public function preExecute() {
 
     // A merge query without any key field is invalid.
     if (count($this->keyFields) == 0) {
       throw new InvalidMergeQueryException("You need to specify key fields before executing a merge query");
     }
 
+    return TRUE;
+  }
+
+  public function execute() {
+    // If validation fails, simply return NULL.
+    // Note that validation routines in preExecute() may throw exceptions instead.
+    if (!$this->preExecute()) {
+      return NULL;
+    }
+
     // In the degenerate case of this query type, we have to run multiple
     // queries as there is no universal single-query mechanism that will work.
     // Our degenerate case is not designed for performance efficiency but
diff --git a/includes/database/select.inc b/includes/database/select.inc
index 449ab11f4..073f82c88 100644
--- a/includes/database/select.inc
+++ b/includes/database/select.inc
@@ -359,6 +359,19 @@ interface SelectQueryInterface extends QueryConditionInterface, QueryAlterableIn
   public function countQuery();
 
   /**
+   * Indicates if preExecute() has already been called on that object.
+   */
+  public function isPrepared();
+
+  /**
+   * Generic preparation and validation for a SELECT query.
+   *
+   * @return
+   *   TRUE if the validation was successful, FALSE if not.
+   */
+  public function preExecute(SelectQueryInterface $query = NULL);
+
+  /**
    * Clone magic method.
    *
    * Select queries have dependent objects that must be deep-cloned.  The
@@ -501,7 +514,27 @@ class SelectQueryExtender implements SelectQueryInterface {
     return $this->query->getArguments();
   }
 
+  public function isPrepared() {
+    return $this->query->isPrepared();
+  }
+
+  public function preExecute(SelectQueryInterface $query = NULL) {
+    // If no query object is passed in, use $this.
+    if (is_null($query)) {
+      $query = $this;
+    }
+
+    return $this->query->preExecute($query);
+  }
+
   public function execute() {
+    // By calling preExecute() here, we force it to preprocess the extender
+    // object rather than just the base query object.  That means
+    // hook_query_alter() gets access to the extended object.
+    if (!$this->preExecute($this)) {
+      return NULL;
+    }
+
     return $this->query->execute();
   }
 
@@ -718,6 +751,12 @@ class SelectQuery extends Query implements SelectQueryInterface {
    */
   protected $range;
 
+  /**
+   * Indicates if preExecute() has already been called.
+   * @var boolean
+   */
+  protected $prepared = FALSE;
+
   public function __construct($table, $alias = NULL, DatabaseConnection $connection, $options = array()) {
     $options['return'] = Database::RETURN_STATEMENT;
     parent::__construct($connection, $options);
@@ -880,14 +919,46 @@ class SelectQuery extends Query implements SelectQueryInterface {
     return $args;
   }
 
-  public function execute() {
+  /**
+   * Indicates if preExecute() has already been called on that object.
+   */
+  public function isPrepared() {
+    return $this->prepared;
+  }
+
+  /**
+   * Generic preparation and validation for a SELECT query.
+   *
+   * @return
+   *   TRUE if the validation was successful, FALSE if not.
+   */
+  public function preExecute(SelectQueryInterface $query = NULL) {
+    // If no query object is passed in, use $this.
+    if (is_null($query)) {
+      $query = $this;
+    }
+
+    // Only execute this once.
+    if ($query->isPrepared()) {
+      return TRUE;
+    }
+
     // Modules may alter all queries or only those having a particular tag.
-    drupal_alter('query', $this);
+    drupal_alter('query', $query);
     if (isset($this->alterTags)) {
       foreach ($this->alterTags as $tag => $value) {
-        drupal_alter("query_$tag", $this);
+        drupal_alter("query_$tag", $query);
       }
     }
+    return $this->prepared = TRUE;
+  }
+
+  public function execute() {
+    // If validation fails, simply return NULL.
+    // Note that validation routines in preExecute() may throw exceptions instead.
+    if (!$this->preExecute()) {
+      return NULL;
+    }
 
     $args = $this->getArguments();
 
diff --git a/includes/pager.inc b/includes/pager.inc
index 26b9eb833..e9e6ea257 100644
--- a/includes/pager.inc
+++ b/includes/pager.inc
@@ -43,6 +43,14 @@ class PagerDefault extends SelectQueryExtender {
    */
   protected $customCountQuery = FALSE;
 
+  public function __construct(SelectQueryInterface $query, DatabaseConnection $connection) {
+    parent::__construct($query, $connection);
+
+    // Add pager tag. Do this here to ensure that it is always added before
+    // preExecute() is called.
+    $this->addTag('pager');
+  }
+
   /**
    * Override the execute method.
    *
@@ -52,6 +60,13 @@ class PagerDefault extends SelectQueryExtender {
   public function execute() {
     global $pager_page_array, $pager_total, $pager_total_items, $pager_limits;
 
+    // Add convenience tag to mark that this is an extended query. We have to
+    // do this in the constructor to ensure that it is set before preExecute()
+    // gets called.
+    if (!$this->preExecute($this)) {
+      return NULL;
+    }
+
     // A NULL limit is the "kill switch" for pager queries.
     if (empty($this->limit)) {
       return;
diff --git a/includes/tablesort.inc b/includes/tablesort.inc
index 8259a9af4..5b17f9c94 100644
--- a/includes/tablesort.inc
+++ b/includes/tablesort.inc
@@ -22,10 +22,22 @@ class TableSort extends SelectQueryExtender {
    */
   protected $header = array();
 
-  public function execute() {
-    return $this->query->execute();
+  public function __construct(SelectQueryInterface $query, DatabaseConnection $connection) {
+    parent::__construct($query, $connection);
+
+    // Add convenience tag to mark that this is an extended query. We have to
+    // do this in the constructor to ensure that it is set before preExecute()
+    // gets called.
+    $this->addTag('tablesort');
   }
 
+  /**
+   * Order the query based on a header array.
+   *
+   * @see theme_table()
+   * @param $header
+   *   Table header array.
+   */
   public function orderByHeader(Array $header) {
     $this->header = $header;
     $ts = $this->init();