- Patch #908798 by bojanz: EntityFieldQuery does not handle ordering correctly.

3 files changed, 108 insertions, 91 deletions

diff --git a/includes/entity.inc b/includes/entity.inc
index b67a31798..2a7880bb1 100644
--- a/includes/entity.inc
+++ b/includes/entity.inc
@@ -446,31 +446,11 @@ class EntityFieldQuery {
   public $propertyConditions = array();
 
   /**
-   * List of order clauses for entity-generic metadata.
+   * List of order clauses.
    *
    * @var array
-   *
-   * @see EntityFieldQuery::entityOrderBy()
-   */
-  public $entityOrder = array();
-
-  /**
-   * List of order clauses for fields.
-   *
-   * @var array
-   *
-   * @see EntityFieldQuery::fieldOrderBy()
-   */
-  public $fieldOrder = array();
-
-  /**
-   * List of order clauses for entities.
-   *
-   * @var array
-   *
-   * @see EntityFieldQuery::entityOrderBy()
    */
-  public $propertyOrder = array();
+  public $order = array();
 
   /**
    * The query range.
@@ -709,7 +689,11 @@ class EntityFieldQuery {
    *   The called object.
    */
   public function entityOrderBy($name, $direction) {
-    $this->entityOrder[$name] = $direction;
+    $this->order[] = array(
+      'type' => 'entity',
+      'specifier' => $name,
+      'direction' => $direction,
+    );
     return $this;
   }
 
@@ -734,12 +718,16 @@ class EntityFieldQuery {
     if (is_scalar($field)) {
       $field = field_info_field($field);
     }
-    // Ensure the same index is used for fieldOrder as for fields.
+    // Save the index used for the new field, for later use in field storage.
     $index = count($this->fields);
     $this->fields[$index] = $field;
-    $this->fieldOrder[$index] = array(
-      'field' => $field,
-      'column' => $column,
+    $this->order[] = array(
+      'type' => 'field',
+      'specifier' => array(
+        'field' => $field,
+        'index' => $index,
+        'column' => $column,
+      ),
       'direction' => $direction,
     );
     return $this;
@@ -764,8 +752,9 @@ class EntityFieldQuery {
    *   The called object.
    */
   public function propertyOrderBy($column, $direction) {
-    $this->propertyOrder[] = array(
-      'column' => $column,
+    $this->order[] = array(
+      'type' => 'property',
+      'specifier' => $column,
       'direction' => $direction,
     );
     return $this;
@@ -919,13 +908,6 @@ class EntityFieldQuery {
     // Execute the query using the correct callback.
     $result = call_user_func($this->queryCallback(), $this);
 
-    // Sanity checks.
-    if (!empty($this->propertyConditions)) {
-      throw new EntityFieldQueryException(t('Property query conditions were not handled in !function.', array('!function' => $function)));
-    }
-    if (!empty($this->propertyOrderBy)) {
-      throw new EntityFieldQueryException(t('Property query order by was not handled in !function.', array('!function' => $function)));
-    }
     return $result;
   }
 
@@ -985,6 +967,10 @@ class EntityFieldQuery {
     $base_table = $entity_info['base table'];
     $select_query = db_select($base_table);
     $select_query->addExpression(':entity_type', 'entity_type', array(':entity_type' => $entity_type));
+    // Process the property conditions.
+    foreach ($this->propertyConditions as $property_condition) {
+      $this->addCondition($select_query, "$base_table." . $property_condition['column'], $property_condition);
+    }
     // Process the four possible entity condition.
     // The id field is always present in entity keys.
     $sql_field = $entity_info['entity keys']['id'];
@@ -1024,15 +1010,20 @@ class EntityFieldQuery {
       $this->addCondition($select_query, $sql_field, $this->entityConditions['bundle'], $having);
     }
 
-    foreach ($this->entityOrder as $key => $direction) {
-      if (isset($id_map[$key])) {
-        $select_query->orderBy($id_map[$key], $direction);
+    // Order the query.
+    foreach ($this->order as $order) {
+      if ($order['type'] == 'entity') {
+        $key = $order['specifier'];
+        if (!isset($id_map[$key])) {
+          throw new EntityFieldQueryException(t('Do not know how to order on @key for @entity_type', array('@key' => $key, '@entity_type' => $entity_type)));
+        }
+        $select_query->orderBy($id_map[$key], $order['direction']);
       }
-      else {
-        throw new EntityFieldQueryException(t('Do not know how to order on @key for @entity_type', array('@key' => $key, '@entity_type' => $entity_type)));
+      elseif ($order['type'] == 'property') {
+        $select_query->orderBy("$base_table." . $order['specifier'], $order['direction']);
       }
     }
-    $this->processProperty($select_query, $base_table);
+
     return $this->finishQuery($select_query);
   }
 
@@ -1074,27 +1065,6 @@ class EntityFieldQuery {
   }
 
   /**
-   * Processes the property condition and orders.
-   *
-   * This is a helper for hook_entity_query() and hook_field_storage_query().
-   *
-   * @param SelectQuery $select_query
-   *   A SelectQuery object.
-   * @param $entity_base_table
-   *   The name of the entity base table. This already should be in
-   *   $select_query.
-   */
-  public function processProperty(SelectQuery $select_query, $entity_base_table) {
-    foreach ($this->propertyConditions as $entity_condition) {
-      $this->addCondition($select_query, "$entity_base_table." . $entity_condition['column'], $entity_condition);
-    }
-    foreach ($this->propertyOrder as $order) {
-      $select_query->orderBy("$entity_base_table." . $order['column'], $order['direction']);
-    }
-    unset($this->propertyConditions, $this->propertyOrder);
-  }
-
-  /**
    * Adds a condition to an already built SelectQuery (internal function).
    *
    * This is a helper for hook_entity_query() and hook_field_storage_query().
diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.module b/modules/field/modules/field_sql_storage/field_sql_storage.module
index 9349a95d6..19401fcb4 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.module
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -550,35 +550,53 @@ function field_sql_storage_field_storage_query(EntityFieldQuery $query) {
     }
   }
 
-  // Add field orders.
-  foreach ($query->fieldOrder as $key => $order) {
-    $table_alias = $table_aliases[$key];
-    $field = $order['field'];
-    $sql_field = "$table_alias." . _field_sql_storage_columnname($field['field_name'], $order['column']);
-    $select_query->orderBy($sql_field, $order['direction']);
-  }
-
   if (isset($query->deleted)) {
     $select_query->condition("$field_base_table.deleted", (int) $query->deleted);
   }
 
-  if ($query->propertyConditions || $query->propertyOrder) {
+  // Is there a need to sort the query by property?
+  $has_property_order = FALSE;
+  foreach ($query->order as $order) {
+    if ($order['type'] == 'property') {
+      $has_property_order = TRUE;
+    }
+  }
+
+  if ($query->propertyConditions || $has_property_order) {
     if (empty($query->entityConditions['entity_type']['value'])) {
       throw new EntityFieldQueryException('Property conditions and orders must have an entity type defined.');
     }
     $entity_type = $query->entityConditions['entity_type']['value'];
     $entity_base_table = _field_sql_storage_query_join_entity($select_query, $entity_type, $field_base_table);
     $query->entityConditions['entity_type']['operator'] = '=';
-    $query->processProperty($select_query, $entity_base_table);
+    foreach ($query->propertyConditions as $property_condition) {
+      $query->addCondition($select_query, "$entity_base_table." . $property_condition['column'], $property_condition);
+    }
   }
   foreach ($query->entityConditions as $key => $condition) {
     $sql_field = $key == 'entity_type' ? 'fcet.type' : "$field_base_table.$key";
     $query->addCondition($select_query, $sql_field, $condition);
   }
-  foreach ($query->entityOrder as $key => $direction) {
-    $sql_field = $key == 'entity_type' ? 'fcet.type' : "$field_base_table.$key";
-    $select_query->orderBy($sql_field, $direction);
+
+  // Order the query.
+  foreach ($query->order as $order) {
+    if ($order['type'] == 'entity') {
+      $key = $order['specifier'];
+      $sql_field = $key == 'entity_type' ? 'fcet.type' : "$field_base_table.$key";
+      $select_query->orderBy($sql_field, $order['direction']);
+    }
+    elseif ($order['type'] == 'field') {
+      $specifier = $order['specifier'];
+      $field = $specifier['field'];
+      $table_alias = $table_aliases[$specifier['index']];
+      $sql_field = "$table_alias." . _field_sql_storage_columnname($field['field_name'], $specifier['column']);
+      $select_query->orderBy($sql_field, $order['direction']);
+    }
+    elseif ($order['type'] == 'property') {
+      $select_query->orderBy("$entity_base_table." . $order['specifier'], $order['direction']);
+    }
   }
+
   return $query->finishQuery($select_query, $id_key);
 }
 
diff --git a/modules/simpletest/tests/entity_query.test b/modules/simpletest/tests/entity_query.test
index e569cba1a..171c51730 100644
--- a/modules/simpletest/tests/entity_query.test
+++ b/modules/simpletest/tests/entity_query.test
@@ -295,7 +295,7 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
       array('test_entity_bundle_key', 2),
       array('test_entity_bundle_key', 1),
     ), t('Test sort entity entity_id in descending order.'), TRUE);
-    
+
     // Test entity sort by entity_id, with a field condition.
     $query = new EntityFieldQuery();
     $query
@@ -351,7 +351,7 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
       array('test_entity_bundle_key', 2),
       array('test_entity_bundle_key', 1),
     ), t('Test sort entity entity_id property in descending order.'), TRUE);
-    
+
     // Test property sort by entity id, with a field condition.
     $query = new EntityFieldQuery();
     $query
@@ -394,7 +394,7 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
       array('test_entity_bundle_key', 1),
       array('test_entity_bundle_key', 6),
       array('test_entity_bundle_key', 5),
-    ), t('Test sort entity bundle in ascending order.'), TRUE);
+    ), t('Test sort entity bundle in ascending order, property in descending order.'), TRUE);
 
     $query = new EntityFieldQuery();
     $query
@@ -408,38 +408,67 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
       array('test_entity_bundle_key', 2),
       array('test_entity_bundle_key', 3),
       array('test_entity_bundle_key', 4),
-    ), t('Test sort entity bundle in descending order.'), TRUE);
-    
+    ), t('Test sort entity bundle in descending order, property in ascending order.'), TRUE);
+
     // Test entity sort by bundle, with a field condition.
     $query = new EntityFieldQuery();
     $query
       ->entityCondition('entity_type', 'test_entity_bundle_key')
       ->fieldCondition($this->fields[0], 'value', 0, '>')
       ->entityOrderBy('bundle', 'ASC')
-      ->propertyOrderBy('ftid', 'ASC');
+      ->propertyOrderBy('ftid', 'DESC');
     $this->assertEntityFieldQuery($query, array(
-      array('test_entity_bundle_key', 1),
-      array('test_entity_bundle_key', 2),
-      array('test_entity_bundle_key', 3),
       array('test_entity_bundle_key', 4),
-      array('test_entity_bundle_key', 5),
+      array('test_entity_bundle_key', 3),
+      array('test_entity_bundle_key', 2),
+      array('test_entity_bundle_key', 1),
       array('test_entity_bundle_key', 6),
-    ), t('Test sort entity bundle in ascending order, with a field condition.'), TRUE);
+      array('test_entity_bundle_key', 5),
+    ), t('Test sort entity bundle in ascending order, property in descending order, with a field condition.'), TRUE);
 
     $query = new EntityFieldQuery();
     $query
       ->entityCondition('entity_type', 'test_entity_bundle_key')
       ->fieldCondition($this->fields[0], 'value', 0, '>')
       ->entityOrderBy('bundle', 'DESC')
-      ->propertyOrderBy('ftid', 'DESC');
+      ->propertyOrderBy('ftid', 'ASC');
     $this->assertEntityFieldQuery($query, array(
-      array('test_entity_bundle_key', 6),
       array('test_entity_bundle_key', 5),
+      array('test_entity_bundle_key', 6),
+      array('test_entity_bundle_key', 1),
+      array('test_entity_bundle_key', 2),
+      array('test_entity_bundle_key', 3),
+      array('test_entity_bundle_key', 4),
+    ), t('Test sort entity bundle in descending order, property in ascending order, with a field condition.'), TRUE);
+
+    // Test entity sort by bundle, field.
+    $query = new EntityFieldQuery();
+    $query
+      ->entityCondition('entity_type', 'test_entity_bundle_key')
+      ->entityOrderBy('bundle', 'ASC')
+      ->fieldOrderBy($this->fields[0], 'value', 'DESC');
+    $this->assertEntityFieldQuery($query, array(
       array('test_entity_bundle_key', 4),
       array('test_entity_bundle_key', 3),
       array('test_entity_bundle_key', 2),
       array('test_entity_bundle_key', 1),
-    ), t('Test sort entity bundle in descending order, with a field condition.'), TRUE);
+      array('test_entity_bundle_key', 6),
+      array('test_entity_bundle_key', 5),
+    ), t('Test sort entity bundle in ascending order, field in descending order.'), TRUE);
+
+    $query = new EntityFieldQuery();
+    $query
+      ->entityCondition('entity_type', 'test_entity_bundle_key')
+      ->entityOrderBy('bundle', 'DESC')
+      ->fieldOrderBy($this->fields[0], 'value', 'ASC');
+    $this->assertEntityFieldQuery($query, array(
+      array('test_entity_bundle_key', 5),
+      array('test_entity_bundle_key', 6),
+      array('test_entity_bundle_key', 1),
+      array('test_entity_bundle_key', 2),
+      array('test_entity_bundle_key', 3),
+      array('test_entity_bundle_key', 4),
+    ), t('Test sort entity bundle in descending order, field in ascending order.'), TRUE);
 
     // Test entity sort by revision_id.
     $query = new EntityFieldQuery();
@@ -463,7 +492,7 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
       array('test_entity', 2),
       array('test_entity', 1),
     ), t('Test sort entity revision_id in descending order.'), TRUE);
-    
+
     // Test entity sort by revision_id, with a field condition.
     $query = new EntityFieldQuery();
     $query