Skip to content

Commit

Permalink
Merge pull request tractorcow-farm#415 from creative-commoners/pulls/…
Browse files Browse the repository at this point in the history 
…4.1/allow-sorting

WIP: FIX Implement localisable order by clause to support translatable sorting
  • Loading branch information
@tractorcow
tractorcow committed Jul 16, 2018
2 parents 988696e + 65e1847 commit 252e489
Show file tree
Hide file tree
Showing 4 changed files with 283 additions and 41 deletions.
103 changes: 76 additions & 27 deletions src/Extension/FluentExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
use SilverStripe\Dev\Deprecation;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\LiteralField;
use SilverStripe\i18n\i18n;
use SilverStripe\ORM\ArrayList;
use SilverStripe\ORM\DataExtension;
Expand Down Expand Up @@ -333,7 +331,7 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
"\"{$table}\".\"ID\" = \"{$joinAlias}\".\"RecordID\" AND \"{$joinAlias}\".\"Locale\" = ?",
$joinAlias,
20,
[ $joinLocale->Locale ]
[$joinLocale->Locale]
);
}
}
Expand All @@ -357,29 +355,15 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)

// Iterate through each select clause, replacing each with the translated version
foreach ($query->getSelect() as $alias => $select) {
// Skip fields without table context
if (!preg_match('/^"(?<table>[\w\\\\]+)"\."(?<field>\w+)"$/i', $select, $matches)) {
continue;
}

$table = $matches['table'];
$field = $matches['field'];

// If this table doesn't have translated fields then skip
if (empty($tables[$table])) {
continue;
}

// If this field shouldn't be translated, skip
if (!in_array($field, $tables[$table])) {
continue;
// Parse fragment for localised field and table
list ($table, $field) = $this->detectLocalisedTableField($tables, $select);
if ($table && $field) {
$expression = $this->localiseSelect($table, $field, $locale);
$query->selectField($expression, $alias);
}

$expression = $this->localiseSelect($table, $field, $locale);
$query->selectField($expression, $alias);
}

// Build all replacements for where conditions
// Build all replacements for where / sort conditions
$conditionSearch = [];
$conditionReplace = [];
foreach ($tables as $table => $fields) {
Expand All @@ -389,9 +373,34 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
}
}

// Iterate through each order clause, replacing each with the translated version
$order = $query->getOrderBy();
foreach ($order as $column => $direction) {
// Parse fragment for localised field and table
list ($table, $field, $fqn) = $this->detectLocalisedTableField($tables, $column);
if ($table && $field) {
$localisedColumn = $column;
// Fix non-fully-qualified name
if (!$fqn) {
$localisedColumn = str_replace(
"\"{$field}\"",
"\"{$table}\".\"{$field}\"",
$localisedColumn
);
}
// Apply substitutions
$localisedColumn = str_replace($conditionSearch, $conditionReplace, $localisedColumn);
if ($column !== $localisedColumn) {
// Wrap sort in group to prevent dataquery messing it up
unset($order[$column]);
$order["({$localisedColumn})"] = $direction;
}
}
}
$query->setOrderBy($order);

// Rewrite where conditions
$where = $query
->getWhere();
$where = $query->getWhere();
foreach ($where as $index => $condition) {
// Extract parameters from condition
if ($condition instanceof SQLConditionGroup) {
Expand All @@ -405,9 +414,9 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
// Apply substitutions
$localisedPredicate = str_replace($conditionSearch, $conditionReplace, $predicate);

$where[$index] = array(
$where[$index] = [
$localisedPredicate => $parameters
);
];
}
$query->setWhere($where);
}
Expand Down Expand Up @@ -926,4 +935,44 @@ protected function requireSavedInLocale()

return $this->owner->config()->get('frontend_publish_required');
}

/**
* Detect a localised field within a SQL fragment.
* Works with either select / sort fragments
*
* If successful, return an array [ thetable, thefield, fqn ]
* Otherwise [ null, null ]
*
* @param array $tables Map of known table and nested fields to search
* @param string $sql The SQL string to inspect
* @return array Three item array with table and field and a flag for whether the fragment is fully quolified
*/
protected function detectLocalisedTableField($tables, $sql)
{
// Check explicit "table"."field" within the fragment
if (preg_match('/"(?<table>[\w\\\\]+)"\."(?<field>\w+)"/i', $sql, $matches)) {
$table = $matches['table'];
$field = $matches['field'];

// Ensure both table and this field are valid
if (empty($tables[$table]) || !in_array($field, $tables[$table])) {
return [null, null, false];
}
return [$table, $field, true];
}

// Check sole "field" without table specifier ("name" without leading or trailing '.')
if (preg_match('/(?<![.])"(?<field>\w+)"(?![.])/i', $sql, $matches)) {
$field = $matches['field'];

// Check if this field is in any of the tables, and just pick any that match
foreach ($tables as $table => $fields) {
if (in_array($field, $fields)) {
return [$table, $field, false];
}
}
}

return [null, null, false];
}
}
169 changes: 156 additions & 13 deletions tests/php/Extension/FluentExtensionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,37 +117,180 @@ public function testWritesToCurrentLocale()
{
FluentState::singleton()->setLocale('en_US');
$record = $this->objFromFixture(LocalisedParent::class, 'record_a');
$this->assertSame(
'en_US',
$this->getLocalisedLocaleFromDb($record),
$this->assertTrue(
$this->hasLocalisedRecord($record, 'en_US'),
'Record can be read from default locale'
);

FluentState::singleton()->setLocale('de_DE');
$record->write();

$record2 = $this->objFromFixture(LocalisedParent::class, 'record_a');
$this->assertSame(
'de_DE',
$this->getLocalisedLocaleFromDb($record2),
'Record Locale is set to current locale'
$this->assertTrue(
$this->hasLocalisedRecord($record2, 'de_DE'),
'Existing record can be read from German locale'
);

// There's no Spanish record yet, so this should create a record in the _Localised table
FluentState::singleton()->setLocale('es_ES');
$record2->Title = 'Un archivo';
$record2->write();

$record3 = $this->objFromFixture(LocalisedParent::class, 'record_a');
$this->assertTrue(
$this->hasLocalisedRecord($record3, 'es_ES'),
'Record Locale is set to current locale when writing new records'
);
}

/**
* Get a Locale field value directly from a record's localised database table, skipping the ORM
*
* @param DataObject $record
* @return string|null
* @param string $locale
* @return boolean
*/
protected function getLocalisedLocaleFromDb(DataObject $record)
protected function hasLocalisedRecord(DataObject $record, $locale)
{
$result = SQLSelect::create()
->setFrom($record->config()->get('table_name') . '_Localised')
->setWhere(['RecordID' => $record->ID])
->setWhere([
'RecordID' => $record->ID,
'Locale' => $locale,
])
->execute()
->first();

return isset($result['Locale']) ? $result['Locale'] : null;
return !empty($result);
}

/**
* Ensure that records can be sorted in their locales
*
* @dataProvider sortRecordProvider
* @param string $locale
* @param string[] $sortArgs
* @param string[] $expected
*/
public function testLocalisedFieldsCanBeSorted($locale, array $sortArgs, $expected)
{
FluentState::singleton()->setLocale($locale);

$records = LocalisedParent::get()->sort(...$sortArgs);
$titles = $records->column('Title');
$this->assertEquals($expected, $titles);
}

/**
* @return array[] Keys: Locale, sorting arguments, expected titles in result
*/
public function sortRecordProvider()
{
return [
/**
* Single field (non-composite) sorting syntax (either string or array syntax)
*
* E.g. `->sort('"foo"')`, `->sort('Title', 'DESC')` etc
*/
'german ascending single sort' => [
'de_DE',
['Title', 'ASC'],
['Eine Akte', 'Lesen Sie mehr', 'Rennen'],
],
'german descending single sort' => [
'de_DE',
['"Title" DESC'],
['Rennen', 'Lesen Sie mehr', 'Eine Akte'],
],
'english ascending single sort' => [
'en_US',
['"Title" ASC'],
['A record', 'Go for a run', 'Read about things'],
],
'english descending single sort' => [
'en_US',
['Title', 'DESC'],
['Read about things', 'Go for a run', 'A record'],
],
'english ascending on unlocalised field' => [
'en_US',
['"Description"'],
['Read about things', 'Go for a run', 'A record'],
],
'english descending on unlocalised field' => [
'en_US',
['"Description" DESC'],
['A record', 'Read about things', 'Go for a run'],
],
'german ascending on unlocalised field' => [
'de_DE',
['"Description"'],
['Lesen Sie mehr', 'Rennen', 'Eine Akte'],
],
'german descending on unlocalised field' => [
'de_DE',
['"Description" DESC'],
['Eine Akte', 'Lesen Sie mehr', 'Rennen'],
],
/**
* Composite sorting tests (either string syntax or array syntax)
*
* E.g. `->sort(['foo' => 'ASC', 'bar' => 'DESC'])`
*/
'english composite sort, string' => [
'en_US',
['"Details" ASC, "Title" ASC'],
['Go for a run', 'A record', 'Read about things']
],
'german composite sort, string' => [
'de_DE',
['"Details" ASC, "Title" ASC'],
['Rennen', 'Eine Akte', 'Lesen Sie mehr'],
],
'english, composite sort, array' => [
'en_US',
[[
'Details' => 'ASC',
'Title' => 'ASC'
]],
['Go for a run', 'A record', 'Read about things'],
],
'german, composite sort, array' => [
'de_DE',
[[
'Details' => 'ASC',
'Title' => 'ASC'
]],
['Rennen', 'Eine Akte', 'Lesen Sie mehr'],
],
'german, composite sort, array (flipped)' => [
'de_DE',
[[
'Details' => 'ASC',
'Title' => 'DESC'
]],
['Rennen', 'Lesen Sie mehr', 'Eine Akte'],
],
'english, composite sort, array (flipped)' => [
'en_US',
[[
'Details' => 'DESC',
'Title' => 'DESC'
]],
['Read about things', 'A record', 'Go for a run'],
],
'german, composite sort, no directions' => [
'de_DE',
['"Details", "Title"'],
['Rennen', 'Eine Akte', 'Lesen Sie mehr'],
],
/**
* Ignored types of sorting, e.g. subqueries. Ignored sorting should use the ORM default
* and sort on whatever is in the base table.
*/
'english, subquery sort' => [
'en_US',
['CONCAT((SELECT COUNT(*) FROM "FluentExtensionTest_LocalisedParent_Localised"), "FluentExtensionTest_LocalisedParent"."ID")'],
['A record', 'Read about things', 'Go for a run'],
]
];
}
}
Loading

0 comments on commit 252e489

Please sign in to comment.