Commit a136ab56 authored by Dries's avatar Dries

- Patch #512236 by yched, bjaspan: design flaw in field_attach_query(),...

- Patch #512236 by yched, bjaspan: design flaw in field_attach_query(), follow-up patch with bugfixes and tests.
parent 6310a1a8
......@@ -701,7 +701,9 @@ function hook_field_attach_pre_update($obj_type, $object, &$skip_fields) {
* A storage module that doesn't support querying a given column should raise
* a FieldQueryException. Incompatibilities should be mentioned on the module
* project page.
* @param $result_format
* @param $count
* See field_attach_query().
* @param $cursor
* See field_attach_query().
* @param $age
* - FIELD_LOAD_CURRENT: query the most recent revisions for all
......@@ -715,7 +717,7 @@ function hook_field_attach_pre_update($obj_type, $object, &$skip_fields) {
* The $skip_field parameter should be set to TRUE if the query has been
* handled.
*/
function hook_field_attach_pre_query($field_name, $conditions, $result_format, $age, &$skip_field) {
function hook_field_attach_pre_query($field_name, $conditions, $count, &$cursor = NULL, $age, &$skip_field) {
}
/**
......@@ -879,14 +881,16 @@ function hook_field_storage_delete_revision($obj_type, $object) {
* A storage module that doesn't support querying a given column should raise
* a FieldQueryException. Incompatibilities should be mentioned on the module
* project page.
* @param $result_format
* @param $count
* See field_attach_query().
* @param $cursor
* See field_attach_query().
* @param $age
* See field_attach_query().
* @return
* See field_attach_query().
*/
function hook_field_storage_query($field_name, $conditions, $result_format, $age) {
function hook_field_storage_query($field_name, $conditions, $count, &$cursor = NULL, $age) {
}
/**
......
......@@ -19,7 +19,7 @@ class FieldValidationException extends FieldException {
* An array of field validation errors, keyed by field name and
* delta that contains two keys:
* - 'error': A machine-readable error code string, prefixed by
* the field module name. A field widget may use this code to decide
* the field module name. A field widget may use this code to decide
* how to report the error.
* - 'message': A human-readable error message such as to be
* passed to form_error() for the appropriate form element.
......@@ -106,11 +106,11 @@ class FieldQueryException extends FieldException {}
*
* field_attach_load(), field_attach_insert(), and
* field_attach_update() also define pre-operation hooks,
* e.g. hook_field_attach_pre_load(). These hooks run before the
* e.g. hook_field_attach_pre_load(). These hooks run before the
* corresponding Field Storage API and Field Type API operations.
* They allow modules to define additional storage locations
* (e.g. denormalizing, mirroring) for field data on a per-field
* basis. They also allow modules to take over field storage
* basis. They also allow modules to take over field storage
* completely by instructing other implementations of the same hook
* and the Field Storage API itself not to operate on specified
* fields.
......@@ -382,15 +382,24 @@ function field_attach_form($obj_type, $object, &$form, &$form_state) {
* fields, or FIELD_LOAD_REVISION to load the version indicated by
* each object. Defaults to FIELD_LOAD_CURRENT; use
* field_attach_load_revision() instead of passing FIELD_LOAD_REVISION.
* @param $options
* An associative array of additional options, with the following keys:
* - 'field_name'
* The field name that should be loaded, instead of loading all fields, for
* each object. Note that returned objects may contain data for other
* fields, for example if they are read from a cache.
* @returns
* Loaded field values are added to $objects. Fields with no values should be
* set as an empty array.
*/
function field_attach_load($obj_type, $objects, $age = FIELD_LOAD_CURRENT) {
function field_attach_load($obj_type, $objects, $age = FIELD_LOAD_CURRENT, $options = array()) {
$load_current = $age == FIELD_LOAD_CURRENT;
$info = field_info_fieldable_types($obj_type);
$cacheable = $load_current && $info['cacheable'];
// Only the most current revision of cacheable fieldable types can be cached.
$cache_read = $load_current && $info['cacheable'];
// In addition, do not write to the cache when loading a single field.
$cache_write = $cache_read && !isset($options['field_name']);
if (empty($objects)) {
return;
......@@ -401,7 +410,7 @@ function field_attach_load($obj_type, $objects, $age = FIELD_LOAD_CURRENT) {
$queried_objects = $objects;
// Fetch available objects from cache, if applicable.
if ($cacheable) {
if ($cache_read) {
// Build the list of cache entries to retrieve.
$cids = array();
foreach ($objects as $id => $object) {
......@@ -434,25 +443,25 @@ function field_attach_load($obj_type, $objects, $age = FIELD_LOAD_CURRENT) {
$skip_fields = array();
foreach (module_implements('field_attach_pre_load') as $module) {
$function = $module . '_field_attach_pre_load';
$function($obj_type, $queried_objects, $age, $skip_fields);
$function($obj_type, $queried_objects, $age, $skip_fields, $options);
}
// Invoke the storage engine's hook_field_storage_load(): the field storage
// engine loads the rest.
module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_load', $obj_type, $queried_objects, $age, $skip_fields);
module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_load', $obj_type, $queried_objects, $age, $skip_fields, $options);
// Invoke field-type module's hook_field_load().
_field_invoke_multiple('load', $obj_type, $queried_objects, $age);
_field_invoke_multiple('load', $obj_type, $queried_objects, $age, $options);
// Invoke hook_field_attach_load(): let other modules act on loading the
// object.
foreach (module_implements('field_attach_load') as $module) {
$function = $module . '_field_attach_load';
$function($obj_type, $queried_objects, $age);
$function($obj_type, $queried_objects, $age, $options);
}
// Build cache data.
if ($cacheable) {
if ($cache_write) {
foreach ($queried_objects as $id => $object) {
$data = array();
list($id, $vid, $bundle) = field_attach_extract_ids($obj_type, $object);
......@@ -480,12 +489,18 @@ function field_attach_load($obj_type, $objects, $age = FIELD_LOAD_CURRENT) {
* An array of objects for which to load fields, keyed by object id.
* Each object needs to have its 'bundle', 'id' and (if applicable)
* 'revision' keys filled.
* @param $options
* An associative array of additional options, with the following keys:
* - 'field_name'
* The field name that should be loaded, instead of loading all fields, for
* each object. Note that returned objects may contain data for other
* fields, for example if they are read from a cache.
* @returns
* On return, the objects in $objects are modified by having the
* appropriate set of fields added.
*/
function field_attach_load_revision($obj_type, $objects) {
return field_attach_load($obj_type, $objects, FIELD_LOAD_REVISION);
function field_attach_load_revision($obj_type, $objects, $options = array()) {
return field_attach_load($obj_type, $objects, FIELD_LOAD_REVISION, $options);
}
/**
......@@ -778,7 +793,9 @@ function field_attach_delete_revision($obj_type, $object) {
* @param &$cursor
* An opaque cursor that allows a caller to iterate through multiple
* result sets. On the first call, pass 0; the correct value to pass
* on the next call will be written into $cursor on return. If NULL,
* on the next call will be written into $cursor on return. When
* there is no more query data available, $cursor will be filled in
* with FIELD_QUERY_COMPLETE. If $cursor is passed as NULL,
* the first result set is returned and no next cursor is returned.
* @param $age
* Internal use only. Use field_attach_query_revisions() instead of passing
......@@ -814,7 +831,8 @@ function field_attach_query($field_name, $conditions, $count, &$cursor = NULL, $
}
// If the request hasn't been handled, let the storage engine handle it.
if (!$skip_field) {
$results = module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_query', $field_name, $conditions, $count, $cursor, $age);
$function = variable_get('field_storage_module', 'field_sql_storage') . '_field_storage_query';
$results = $function($field_name, $conditions, $count, $cursor, $age);
}
return $results;
......
......@@ -100,14 +100,10 @@
define('FIELD_QUERY_NO_LIMIT', 'FIELD_QUERY_NO_LIMIT');
/**
* Result format argument for field_attach_query().
* Cursor return value for field_attach_query() to indicate that no
* more data is available.
*/
define('FIELD_QUERY_RETURN_VALUES', 'FIELD_QUERY_RETURN_VALUES');
/**
* Result format argument for field_attach_query().
*/
define('FIELD_QUERY_RETURN_IDS', 'FIELD_QUERY_RETURN_IDS');
define('FIELD_QUERY_COMPLETE', 'FIELD_QUERY_COMPLETE');
/**
* @} End of "Field query flags".
......
......@@ -173,6 +173,14 @@ class FieldAttachTestCase extends DrupalWebTestCase {
$this->assertEqual($entity->{$field_name}[0]['additional_key'], 'additional_value', t('Entity %index: extra information was found', array('%index' => $index)));
}
}
// Check that the single-field load option works.
$entity = field_test_create_stub_entity(1, 1, $bundles[1]);
field_attach_load($entity_type, array(1 => $entity), FIELD_LOAD_CURRENT, array('field_name' => $field_names[1]));
$this->assertEqual($entity->{$field_names[1]}[0]['value'], $values[1][$field_names[1]], t('Entity %index: expected value was found.', array('%index' => 1)));
$this->assertEqual($entity->{$field_names[1]}[0]['additional_key'], 'additional_value', t('Entity %index: extra information was found', array('%index' => 1)));
$this->assert(!isset($entity->{$field_names[2]}), t('Entity %index: field %field_name is not loaded.', array('%index' => 2, '%field_name' => $field_names[2])));
$this->assert(!isset($entity->{$field_names[3]}), t('Entity %index: field %field_name is not loaded.', array('%index' => 3, '%field_name' => $field_names[3])));
}
/**
......@@ -328,7 +336,7 @@ class FieldAttachTestCase extends DrupalWebTestCase {
$result = field_attach_query($this->field_name, $conditions, FIELD_QUERY_NO_LIMIT);
$this->assertTrue(isset($result[$entity_types[1]][1]) && !isset($result[$entity_types[2]][2]), t("Query on a value common to both objects and an 'entity_id' condition only returns the relevant object"));
// Test FIELD_QUERY_RETURN_IDS result format.
// Test result format.
$conditions = array(array('value', $values[0]));
$result = field_attach_query($this->field_name, $conditions, FIELD_QUERY_NO_LIMIT);
$expected = array(
......@@ -336,7 +344,47 @@ class FieldAttachTestCase extends DrupalWebTestCase {
$entities[1]->ftid => field_test_create_stub_entity($entities[1]->ftid, $entities[1]->ftvid),
)
);
$this->assertEqual($result, $expected, t('FIELD_QUERY_RETURN_IDS result format returns the expect result'));
$this->assertEqual($result, $expected, t('Result format is correct.'));
// Now test the count/offset paging capability.
// Create a new bundle with an instance of the field.
field_test_create_bundle('offset_bundle', 'Offset Test Bundle');
$this->instance2 = $this->instance;
$this->instance2['bundle'] = 'offset_bundle';
field_create_instance($this->instance2);
// Create 20 test objects, using the new bundle, but with
// non-sequential ids so we can tell we are getting the right ones
// back. We do not need unique values since field_attach_query()
// won't return them anyway.
$offset_entities = array();
$offset_id = mt_rand(1, 3);
for ($i = 0; $i < 20; ++$i) {
$offset_id += mt_rand(2, 5);
$offset_entities[$offset_id] = field_test_create_stub_entity($offset_id, $offset_id, 'offset_bundle');
$offset_entities[$offset_id]->{$this->field_name}[0] = array('value' => $offset_id);
field_attach_insert('test_entity', $offset_entities[$offset_id]);
}
// Query for the offset entities in batches, making sure we get
// back the right ones.
$cursor = 0;
foreach (array(1 => 1, 3 => 3, 5 => 5, 8 => 8, 13 => 3) as $count => $expect) {
$found = field_attach_query($this->field_name, array(array('bundle', 'offset_bundle')), $count, $cursor);
if (isset($found['test_entity'])) {
$this->assertEqual(count($found['test_entity']), $expect, t('Requested @count, expected @expect, got @found, cursor @cursor', array('@count' => $count, '@expect' => $expect, '@found' => count($found['test_entity']), '@cursor' => $cursor)));
foreach ($found['test_entity'] as $id => $entity) {
$this->assert(isset($offset_entities[$id]), "Entity $id found");
unset($offset_entities[$id]);
}
}
else {
$this->assertEqual(0, $expect, t('Requested @count, expected @expect, got @found, cursor @cursor', array('@count' => $count, '@expect' => $expect, '@found' => 0, '@cursor' => $cursor)));
}
}
$this->assertEqual(count($offset_entities), 0, "All entities found");
$this->assertEqual($cursor, FIELD_QUERY_COMPLETE, "Cursor is FIELD_QUERY_COMPLETE");
}
/**
......
......@@ -202,7 +202,7 @@ function field_sql_storage_field_storage_delete_field($field_name) {
/**
* Implement hook_field_storage_load().
*/
function field_sql_storage_field_storage_load($obj_type, $objects, $age, $skip_fields = array()) {
function field_sql_storage_field_storage_load($obj_type, $objects, $age, $skip_fields, $options) {
$etid = _field_sql_storage_etid($obj_type);
$load_current = $age == FIELD_LOAD_CURRENT;
......@@ -212,7 +212,7 @@ function field_sql_storage_field_storage_load($obj_type, $objects, $age, $skip_f
foreach ($objects as $obj) {
list($id, $vid, $bundle) = field_attach_extract_ids($obj_type, $obj);
foreach (field_info_instances($bundle) as $field_name => $instance) {
if (!isset($skip_fields[$field_name])) {
if (!isset($skip_fields[$field_name]) && (!isset($options['field_name']) || $options['field_name'] == $instance['field_name'])) {
$objects[$id]->{$field_name} = array();
$field_ids[$field_name][] = $load_current ? $id : $vid;
$delta_count[$id][$field_name] = 0;
......@@ -224,13 +224,14 @@ function field_sql_storage_field_storage_load($obj_type, $objects, $age, $skip_f
$field = field_info_field($field_name);
$table = $load_current ? _field_sql_storage_tablename($field) : _field_sql_storage_revision_tablename($field);
$results = db_select($table, 't')
$query = db_select($table, 't')
->fields('t')
->condition('etid', $etid)
->condition($load_current ? 'entity_id' : 'revision_id', $ids, 'IN')
->condition('deleted', 0)
->orderBy('delta')
->execute();
->orderBy('delta');
$results = $query->execute();
foreach ($results as $row) {
if ($field['cardinality'] == FIELD_CARDINALITY_UNLIMITED || $delta_count[$row->entity_id][$field_name] < $field['cardinality']) {
......@@ -404,35 +405,39 @@ function field_sql_storage_field_storage_query($field_name, $conditions, $count,
// Initialize results array
$return = array();
// Query for batches of rows until we've read $count objects or
// until we get no new rows.
$limit = $count;
// Getting $count objects possibly requires reading more than $count rows
// since fields with multiple values span over several rows. We query for
// batches of $count rows until we've either read $count objects or received
// less rows than asked for.
$obj_count = 0;
do {
if ($limit != FIELD_QUERY_NO_LIMIT) {
$query->range($cursor, $limit);
if ($count != FIELD_QUERY_NO_LIMIT) {
$query->range($cursor, $count);
}
$results = $query->execute();
$found = FALSE;
$row_count = 0;
foreach ($results as $row) {
$found = TRUE;
++$cursor;
$row_count++;
$cursor++;
// If querying all revisions and the entity type has revisions, we need to
// key the results by revision_ids.
// If querying all revisions and the entity type has revisions, we need
// to key the results by revision_ids.
$entity_type = field_info_fieldable_types($row->type);
$id = ($load_current || empty($entity_type['object keys']['revision'])) ? $row->entity_id : $row->revision_id;
// We get multiple rows if the field has multiple deltas. Only
// count the first one.
if (isset($return[$row->type][$id])) {
continue;
if (!isset($return[$row->type][$id])) {
$return[$row->type][$id] = field_attach_create_stub_object($row->type, array($row->entity_id, $row->revision_id, $row->bundle));
$obj_count++;
}
$return[$row->type][$id] = field_attach_create_stub_object($row->type, array($row->entity_id, $row->revision_id, $row->bundle));
--$count;
}
} while ($found && ($limit != FIELD_QUERY_NO_LIMIT || $count > 0));
} while ($count != FIELD_QUERY_NO_LIMIT && $row_count == $count && $obj_count < $count);
// The query is complete when the last batch returns less rows than asked
// for.
if ($row_count < $count) {
$cursor = FIELD_QUERY_COMPLETE;
}
return $return;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment