Skip to content

Commit

Permalink
Fix issue with find(count) and translated conditions.
Browse files Browse the repository at this point in the history 
Because count queries did not have joins created for the translated
fields pagination would generate invalid queries. Checking the conditions
for translated fields and adding in the correct joins solves that.
Extract what would have been duplicated code into methods.

Add a few protected properties to keep method signatures sane. The code
could be even simpler if the existing find(count) join was removed.

Fixes #2349
  • Loading branch information
@markstory
markstory committed Jan 9, 2012
1 parent 9a67a70 commit c548b6b
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 36 deletions.
142 changes: 106 additions & 36 deletions lib/Cake/Model/Behavior/TranslateBehavior.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ class TranslateBehavior extends ModelBehavior {
*/
public $runtime = array();

/**
* Stores the joinTable object for generating joins.
*
* @var object
*/
var $_joinTable;

/**
* Stores the runtime model for generating joins.
*
* @var Model
*/
var $_runtimeModel;

/**
* Callback
*
Expand Down Expand Up @@ -94,6 +108,7 @@ public function beforeFind($model, $query) {
}
$db = $model->getDataSource();
$RuntimeModel = $this->translateModel($model);

if (!empty($RuntimeModel->tablePrefix)) {
$tablePrefix = $RuntimeModel->tablePrefix;
} else {
Expand All @@ -103,8 +118,11 @@ public function beforeFind($model, $query) {
$joinTable->tablePrefix = $tablePrefix;
$joinTable->table = $RuntimeModel->table;

$this->_joinTable = $joinTable;
$this->_runtimeModel = $RuntimeModel;

if (is_string($query['fields']) && 'COUNT(*) AS ' . $db->name('count') == $query['fields']) {
$query['fields'] = 'COUNT(DISTINCT(' . $db->name($model->alias . '.' . $model->primaryKey) . ')) ' . $db->alias . 'count';
$query['fields'] = 'COUNT(DISTINCT('.$db->name($model->alias . '.' . $model->primaryKey) . ')) ' . $db->alias . 'count';
$query['joins'][] = array(
'type' => 'INNER',
'alias' => $RuntimeModel->alias,
Expand All @@ -115,6 +133,11 @@ public function beforeFind($model, $query) {
$RuntimeModel->alias.'.locale' => $locale
)
);
$conditionFields = $this->_checkConditions($model, $query);
foreach ($conditionFields as $field) {
$query = $this->_addJoin($model, $query, $field, $locale);
}
unset($this->_joinTable, $this->_runtimeModel);
return $query;
}

Expand Down Expand Up @@ -144,45 +167,92 @@ public function beforeFind($model, $query) {
unset($query['fields'][$key]);
}
}
$query = $this->_addJoin($model, $query, $field, $locale);
}
}
$this->runtime[$model->alias]['beforeFind'] = $addFields;
unset($this->_joinTable, $this->_runtimeModel);
return $query;
}

if (is_array($locale)) {
foreach ($locale as $_locale) {
$model->virtualFields['i18n_' . $field . '_' . $_locale] = 'I18n__' . $field . '__' . $_locale . '.content';
if (!empty($query['fields'])) {
$query['fields'][] = 'i18n_' . $field . '_' . $_locale;
}
$query['joins'][] = array(
'type' => 'LEFT',
'alias' => 'I18n__' . $field . '__' . $_locale,
'table' => $joinTable,
'conditions' => array(
$model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}__{$_locale}.foreign_key"),
'I18n__' . $field . '__' . $_locale . '.model' => $model->name,
'I18n__' . $field . '__' . $_locale . '.' . $RuntimeModel->displayField => $aliasField,
'I18n__' . $field . '__' . $_locale . '.locale' => $_locale
)
);
}
} else {
$model->virtualFields['i18n_' . $field] = 'I18n__' . $field . '.content';
if (!empty($query['fields'])) {
$query['fields'][] = 'i18n_' . $field;
}
$query['joins'][] = array(
'type' => 'INNER',
'alias' => 'I18n__' . $field,
'table' => $joinTable,
'conditions' => array(
$model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}.foreign_key"),
'I18n__' . $field . '.model' => $model->name,
'I18n__' . $field . '.' . $RuntimeModel->displayField => $aliasField,
'I18n__' . $field . '.locale' => $locale
)
);
/**
* Check a query's conditions for translated fields.
* Return an array of translated fields found in the conditions.
*
* @param Model $model The model being read.
* @param array $query The query array.
* @return array The list of translated fields that are in the conditions.
*/
protected function _checkConditions(Model $model, $query) {
$conditionFields = array();
if (empty($query['conditions']) || (!empty($query['conditions']) && !is_array($query['conditions'])) ) {
return $conditionFields;
}
foreach ($query['conditions'] as $col => $val) {
foreach ($this->settings[$model->alias] as $field => $assoc) {
if (is_numeric($field)) {
$field = $assoc;
}
if (strpos($col, $field) !== false) {
$conditionFields[] = $field;
}
}
}
$this->runtime[$model->alias]['beforeFind'] = $addFields;
return $conditionFields;
}

/**
* Appends a join for translated fields and possibly a field.
*
* @param Model $model The model being worked on.
* @param object $joinTable The jointable object.
* @param array $query The query array to append a join to.
* @param string $field The field name being joined.
* @param mixed $locale The locale(s) having joins added.
* @param boolean $addField Whether or not to add a field.
* @return array The modfied query
*/
protected function _addJoin(Model $model, $query, $field, $locale, $addField = false) {
$db = ConnectionManager::getDataSource($model->useDbConfig);

$RuntimeModel = $this->_runtimeModel;
$joinTable = $this->_joinTable;

if (is_array($locale)) {
foreach ($locale as $_locale) {
$model->virtualFields['i18n_' . $field . '_' . $_locale] = 'I18n__' . $field . '__' . $_locale . '.content';
if (!empty($query['fields']) && is_array($query['fields'])) {
$query['fields'][] = 'i18n_'.$field.'_'.$_locale;
}
$query['joins'][] = array(
'type' => 'LEFT',
'alias' => 'I18n__'.$field.'__'.$_locale,
'table' => $joinTable,
'conditions' => array(
$model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}__{$_locale}.foreign_key"),
'I18n__'.$field.'__'.$_locale.'.model' => $model->name,
'I18n__'.$field.'__'.$_locale.'.'.$RuntimeModel->displayField => $field,
'I18n__'.$field.'__'.$_locale.'.locale' => $_locale
)
);
}
} else {
$model->virtualFields['i18n_' . $field] = 'I18n__' . $field . '.content';
if (!empty($query['fields']) && is_array($query['fields'])) {
$query['fields'][] = 'i18n_'.$field;
}
$query['joins'][] = array(
'type' => 'INNER',
'alias' => 'I18n__'.$field,
'table' => $joinTable,
'conditions' => array(
$model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}.foreign_key"),
'I18n__'.$field.'.model' => $model->name,
'I18n__'.$field.'.'.$RuntimeModel->displayField => $field,
'I18n__'.$field.'.locale' => $locale
)
);
}
return $query;
}

Expand Down
18 changes: 18 additions & 0 deletions lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,24 @@ public function tearDown() {
ClassRegistry::flush();
}

/**
* Test that count queries with conditions get the correct joins
*
* @return void
*/
function testCountWithConditions() {
$this->loadFixtures('Translate', 'TranslatedItem');

$Model =& new TranslatedItem();
$Model->locale = 'eng';
$result = $Model->find('count', array(
'conditions' => array(
'I18n__content.locale' => 'eng'
)
));
$this->assertEqual(3, $result);
}

/**
* testTranslateModel method
*
Expand Down

0 comments on commit c548b6b

Please sign in to comment.