NEW Move index generation to DataObjectSchema and solidify index spec

This commit is contained in:
Daniel Hensby 2017-05-18 17:59:57 +01:00
parent 9fe707d3b9
commit 3e556b5966
No known key found for this signature in database
GPG Key ID: B00D1E9767F0B06E
12 changed files with 281 additions and 162 deletions

View File

@ -405,8 +405,8 @@ abstract class DBSchemaManager
// Create custom indexes
if ($indexSchema) {
foreach ($indexSchema as $indexName => $indexDetails) {
$this->requireIndex($table, $indexName, $indexDetails);
foreach ($indexSchema as $indexName => $indexSpec) {
$this->requireIndex($table, $indexName, $indexSpec);
}
}
}
@ -449,7 +449,6 @@ abstract class DBSchemaManager
$newTable = !isset($this->tableList[strtolower($table)]);
// Force spec into standard array format
$spec = $this->parseIndexSpec($index, $spec);
$specString = $this->convertIndexSpec($spec);
// Check existing index
@ -544,40 +543,6 @@ abstract class DBSchemaManager
}
}
/**
* Converts an array or string index spec into a universally useful array
*
* @see convertIndexSpec() for approximate inverse
* @param string $name Index name
* @param string|array $spec
* @return array The resulting spec array with the required fields name, type, and value
*/
protected function parseIndexSpec($name, $spec)
{
// Support $indexes = array('ColumnName' => true) for quick indexes
if ($spec === true) {
return array(
'name' => $name,
'value' => $this->quoteColumnSpecString($name),
'type' => 'index'
);
}
// Do minimal cleanup on any already parsed spec
if (is_array($spec)) {
$spec['value'] = $this->quoteColumnSpecString($spec['value']);
$spec['type'] = empty($spec['type']) ? 'index' : trim($spec['type']);
return $spec;
}
// Nicely formatted spec!
return array(
'name' => $name,
'value' => $this->quoteColumnSpecString($spec),
'type' => $this->determineIndexType($spec)
);
}
/**
* This takes the index spec which has been provided by a class (ie static $indexes = blah blah)
* and turns it into a proper string.
@ -593,12 +558,12 @@ abstract class DBSchemaManager
protected function convertIndexSpec($indexSpec)
{
// Return already converted spec
if (!is_array($indexSpec)) {
return $indexSpec;
if (!is_array($indexSpec) || !array_key_exists('type', $indexSpec) || !array_key_exists('columns', $indexSpec) || !is_array($indexSpec['columns'])) {
throw new \InvalidArgumentException(sprintf('argument to convertIndexSpec must be correct indexSpec, %s given', var_export($indexSpec, true)));
}
// Combine elements into standard string format
return "{$indexSpec['type']} ({$indexSpec['value']})";
return sprintf('%s (%s)', $indexSpec['type'], $this->implodeColumnList($indexSpec['columns']));
}
/**

View File

@ -39,6 +39,10 @@ class MySQLSchemaManager extends DBSchemaManager
}
if ($indexes) {
foreach ($indexes as $k => $v) {
// force MyISAM if we have a fulltext index
if ($v['type'] === 'fulltext') {
$addOptions = 'ENGINE=MyISAM';
}
$indexSchemas .= $this->getIndexSqlDefinition($k, $v) . ",\n";
}
}
@ -301,19 +305,24 @@ class MySQLSchemaManager extends DBSchemaManager
*/
protected function getIndexSqlDefinition($indexName, $indexSpec)
{
$indexSpec = $this->parseIndexSpec($indexName, $indexSpec);
if ($indexSpec['type'] == 'using') {
return "index \"$indexName\" using ({$indexSpec['value']})";
return sprintf('index "%s" using (%s)', $indexName, $this->implodeColumnList($indexSpec['columns']));
} else {
return "{$indexSpec['type']} \"$indexName\" ({$indexSpec['value']})";
return sprintf('%s "%s" (%s)', $indexSpec['type'], $indexName, $this->implodeColumnList($indexSpec['columns']));
}
}
public function alterIndex($tableName, $indexName, $indexSpec)
{
$indexSpec = $this->parseIndexSpec($indexName, $indexSpec);
$this->query("ALTER TABLE \"$tableName\" DROP INDEX \"$indexName\"");
$this->query("ALTER TABLE \"$tableName\" ADD {$indexSpec['type']} \"$indexName\" {$indexSpec['value']}");
$this->query(sprintf('ALTER TABLE "%s" DROP INDEX "%s"', $tableName, $indexName));
$this->query(sprintf(
'ALTER TABLE "%s" ADD %s "%s" %s',
$tableName,
$indexSpec['type'],
$indexName,
$this->implodeColumnList($indexSpec['columns'])
));
}
protected function indexKey($table, $index, $spec)
@ -347,11 +356,11 @@ class MySQLSchemaManager extends DBSchemaManager
if ($groupedIndexes) {
foreach ($groupedIndexes as $index => $details) {
ksort($details['fields']);
$indexList[$index] = $this->parseIndexSpec($index, array(
$indexList[$index] = array(
'name' => $index,
'value' => $this->implodeColumnList($details['fields']),
'type' => $details['type']
));
'columns' => $details['fields'],
'type' => $details['type'],
);
}
}

View File

@ -3052,38 +3052,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
//-------------------------------------------------------------------------------------------//
/**
* Return the database indexes on this table.
* This array is indexed by the name of the field with the index, and
* the value is the type of index.
*/
public function databaseIndexes()
{
$has_one = $this->uninherited('has_one');
$classIndexes = $this->uninherited('indexes');
//$fileIndexes = $this->uninherited('fileIndexes', true);
$indexes = array();
if ($has_one) {
foreach ($has_one as $relationshipName => $fieldType) {
$indexes[$relationshipName . 'ID'] = true;
}
}
if ($classIndexes) {
foreach ($classIndexes as $indexName => $indexType) {
$indexes[$indexName] = $indexType;
}
}
if (get_parent_class($this) == self::class) {
$indexes['ClassName'] = true;
}
return $indexes;
}
/**
* Check the database schema and update it as necessary.
*
@ -3093,12 +3061,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
{
// Only build the table if we've actually got fields
$schema = static::getSchema();
$fields = $schema->databaseFields(static::class, false);
$table = $schema->tableName(static::class);
$fields = $schema->databaseFields(static::class, false);
$indexes = $schema->databaseIndexes(static::class, false);
$extensions = self::database_extensions(static::class);
$indexes = $this->databaseIndexes();
if (empty($table)) {
throw new LogicException(
"Class " . static::class . " not loaded by manifest, or no database table configured"
@ -3144,10 +3111,18 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
}
// Build index list
$manymanyIndexes = array(
$parentField => true,
$childField => true,
);
$manymanyIndexes = [
$parentField => [
'type' => 'index',
'name' => $parentField,
'columns' => [$parentField],
],
$childField => [
'type' => 'index',
'name' =>$childField,
'columns' => [$childField],
],
];
DB::require_table($tableOrClass, $manymanyFields, $manymanyIndexes, true, null, $extensions);
}
}

View File

@ -36,6 +36,13 @@ class DataObjectSchema
*/
protected $databaseFields = [];
/**
* Cache of database indexes
*
* @var array
*/
protected $databaseIndexes = [];
/**
* Cache of composite database field
*
@ -57,6 +64,7 @@ class DataObjectSchema
{
$this->tableNames = [];
$this->databaseFields = [];
$this->databaseIndexes = [];
$this->compositeFields = [];
}
@ -333,6 +341,26 @@ class DataObjectSchema
return isset($fields[$field]) ? $fields[$field] : null;
}
/**
* @param string $class
* @param bool $aggregated
*
* @return array
*/
public function databaseIndexes($class, $aggregated = true)
{
$class = ClassInfo::class_name($class);
if ($class === DataObject::class) {
return [];
}
$this->cacheDatabaseIndexes($class);
$indexes = $this->databaseIndexes[$class];
if (!$aggregated) {
return $indexes;
}
return array_merge($indexes, $this->databaseIndexes(get_parent_class($class)));
}
/**
* Check if the given class has a table
*
@ -456,6 +484,70 @@ class DataObjectSchema
$this->compositeFields[$class] = $compositeFields;
}
/**
* Cache all indexes for the given class.
* Will do nothing if already cached
*
* @param $class
*/
protected function cacheDatabaseIndexes($class)
{
if (array_key_exists($class, $this->databaseIndexes)) {
return;
}
$indexes = [];
// look for indexable field types
foreach ($this->databaseFields($class, false) as $field => $type) {
if ($type === 'ForeignKey' || $type === 'DBClassName') {
$indexes[$field] = [
'type' => 'index',
'columns' => [$field],
];
}
}
// look for custom indexes declared on the class
$classIndexes = Config::inst()->get($class, 'indexes', Config::UNINHERITED) ?: [];
foreach ($classIndexes as $indexName => $indexSpec) {
if (array_key_exists($indexName, $indexes)) {
throw new InvalidArgumentException(sprintf(
'Index named "%s" already exists on class %s',
$indexName,
$class
));
}
if (is_array($indexSpec)) {
if (!ArrayLib::is_associative($indexSpec)) {
$indexSpec = [
'columns' => $indexSpec,
];
}
if (!isset($indexSpec['type'])) {
$indexSpec['type'] = 'index';
}
if (!isset($indexSpec['columns'])) {
$indexSpec['columns'] = [$indexName];
} elseif (!is_array($indexSpec['columns'])) {
throw new InvalidArgumentException(sprintf(
'Index %s on %s is not valid. columns should be an array %s given',
var_export($indexName, true),
var_export($class, true),
var_export($indexSpec['columns'], true)
));
}
} else {
$indexSpec = [
'type' => 'index',
'columns' => [$indexName],
];
}
$indexes[$indexName] = $indexSpec;
}
$this->databaseIndexes[$class] = $indexes;
}
/**
* Returns the table name in the class hierarchy which contains a given
* field column for a {@link DataObject}. If the field does not exist, this

View File

@ -2,9 +2,9 @@
namespace SilverStripe\ORM\Filters;
use SilverStripe\Core\Convert;
use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\DataObject;
use SilverStripe\Core\Config\Config;
use Exception;
/**
@ -63,27 +63,21 @@ class FulltextFilter extends SearchFilter
*/
public function getDbName()
{
$indexes = Config::inst()->get($this->model, "indexes");
if (is_array($indexes) && array_key_exists($this->getName(), $indexes)) {
$indexes = DataObject::getSchema()->databaseIndexes($this->model);
if (array_key_exists($this->getName(), $indexes)) {
$index = $indexes[$this->getName()];
if (is_array($index) && array_key_exists("value", $index)) {
return $this->prepareColumns($index['value']);
} else {
// Parse a fulltext string (eg. fulltext ("ColumnA", "ColumnB")) to figure out which columns
// we need to search.
if (preg_match('/^fulltext\s+\((.+)\)$/i', $index, $matches)) {
return $this->prepareColumns($matches[1]);
} else {
throw new Exception(sprintf(
"Invalid fulltext index format for '%s' on '%s'",
$this->getName(),
$this->model
));
}
}
} else {
return parent::getDbName();
}
if (is_array($index) && array_key_exists('columns', $index)) {
return $this->prepareColumns($index['columns']);
} else {
throw new Exception(sprintf(
"Invalid fulltext index format for '%s' on '%s'",
var_export($this->getName(), true),
var_export($this->model, true)
));
}
return parent::getDbName();
}
/**
@ -95,11 +89,10 @@ class FulltextFilter extends SearchFilter
*/
protected function prepareColumns($columns)
{
$cols = preg_split('/"?\s*,\s*"?/', trim($columns, '(") '));
$table = DataObject::getSchema()->tableForField($this->model, current($cols));
$cols = array_map(function ($col) use ($table) {
return sprintf('"%s"."%s"', $table, $col);
}, $cols);
return implode(',', $cols);
$table = DataObject::getSchema()->tableForField($this->model, current($columns));
$columns = array_map(function ($column) use ($table) {
return Convert::symbol2sql("$table.$column");
}, $columns);
return implode(',', $columns);
}
}

View File

@ -56,8 +56,8 @@ class FulltextSearchable extends DataExtension
public static function enable($searchableClasses = [SiteTree::class, File::class])
{
$defaultColumns = array(
SiteTree::class => '"Title","MenuTitle","Content","MetaDescription"',
File::class => '"Name","Title"'
SiteTree::class => ['Title','MenuTitle','Content','MetaDescription'],
File::class => ['Name','Title'],
);
if (!is_array($searchableClasses)) {
@ -69,12 +69,7 @@ class FulltextSearchable extends DataExtension
}
if (isset($defaultColumns[$class])) {
Config::modify()->set(
$class,
'create_table_options',
array(MySQLSchemaManager::ID => 'ENGINE=MyISAM')
);
$class::add_extension(__CLASS__."('{$defaultColumns[$class]}')");
$class::add_extension(sprintf('%s(%s)', static::class, "'" . implode("','", $defaultColumns[$class]) . "''"));
} else {
throw new Exception(
"FulltextSearchable::enable() I don't know the default search columns for class '$class'"
@ -94,9 +89,12 @@ class FulltextSearchable extends DataExtension
public function __construct($searchFields = array())
{
if (is_array($searchFields)) {
$this->searchFields = '"'.implode('","', $searchFields).'"';
} else {
$this->searchFields = $searchFields;
} else {
$this->searchFields = explode(',', $searchFields);
foreach ($this->searchFields as &$field) {
$field = trim($field);
}
}
}
@ -107,7 +105,7 @@ class FulltextSearchable extends DataExtension
'SearchFields' => array(
'type' => 'fulltext',
'name' => 'SearchFields',
'value' => $args[0]
'columns' => $args,
)
)
);

View File

@ -175,11 +175,11 @@ class DataObjectSchemaGenerationTest extends SapphireTest
// Update the SearchFields index here
TestIndexObject::config()->update(
'indexes',
array(
'SearchFields' => array(
'value' => 'Title'
)
)
[
'SearchFields' => [
'columns' => ['Title'],
],
]
);
// Verify that the above index change triggered a schema update

View File

@ -7,29 +7,35 @@ use SilverStripe\Dev\TestOnly;
class TestIndexObject extends TestObject implements TestOnly
{
private static $table_name = 'DataObjectSchemaGenerationTest_IndexDO';
private static $db = array(
private static $db = [
'Title' => 'Varchar(255)',
'Content' => 'Text'
);
'Content' => 'Text',
];
private static $indexes = array(
'NameIndex' => 'unique ("Title")',
'SearchFields' => array(
private static $indexes = [
'NameIndex' => [
'type' => 'unique',
'columns' => ['Title'],
],
'SearchFields' => [
'type' => 'fulltext',
'name' => 'SearchFields',
'value' => '"Title","Content"'
)
);
'columns' => ['Title', 'Content'],
],
];
/**
* @config
*/
private static $indexes_alt = array(
'NameIndex' => array(
* @config
*/
private static $indexes_alt = [
'NameIndex' => [
'type' => 'unique',
'name' => 'NameIndex',
'value' => '"Title"'
),
'SearchFields' => 'fulltext ("Title","Content")'
);
'columns' => ['Title'],
],
'SearchFields' => [
'type' => 'fulltext',
'columns' => ['Title', 'Content'],
],
];
}

View File

@ -6,6 +6,7 @@ use SilverStripe\Core\ClassInfo;
use SilverStripe\ORM\DataObject;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataObjectSchema;
use SilverStripe\ORM\Tests\DataObjectSchemaTest\AllIndexes;
use SilverStripe\ORM\Tests\DataObjectSchemaTest\BaseClass;
use SilverStripe\ORM\Tests\DataObjectSchemaTest\BaseDataClass;
use SilverStripe\ORM\Tests\DataObjectSchemaTest\ChildClass;
@ -23,7 +24,7 @@ use SilverStripe\ORM\Tests\DataObjectSchemaTest\WithRelation;
*/
class DataObjectSchemaTest extends SapphireTest
{
protected static $extra_dataobjects = array(
protected static $extra_dataobjects = [
// Classes in base namespace
BaseClass::class,
BaseDataClass::class,
@ -33,8 +34,9 @@ class DataObjectSchemaTest extends SapphireTest
NoFields::class,
WithCustomTable::class,
WithRelation::class,
DefaultTableName::class
);
DefaultTableName::class,
AllIndexes::class,
];
/**
* Test table name generation
@ -234,4 +236,39 @@ class DataObjectSchemaTest extends SapphireTest
$this->setExpectedException('InvalidArgumentException');
$schema->baseDataClass(DataObject::class);
}
public function testDatabaseIndexes()
{
$indexes = DataObject::getSchema()->databaseIndexes(AllIndexes::class);
$this->assertCount(5, $indexes);
$this->assertArrayHasKey('ClassName', $indexes);
$this->assertEquals([
'type' => 'index',
'columns' => ['ClassName'],
], $indexes['ClassName']);
$this->assertArrayHasKey('Content', $indexes);
$this->assertEquals([
'type' => 'index',
'columns' => ['Content'],
], $indexes['Content']);
$this->assertArrayHasKey('IndexCols', $indexes);
$this->assertEquals([
'type' => 'index',
'columns' => ['Title', 'Content'],
], $indexes['IndexCols']);
$this->assertArrayHasKey('IndexUnique', $indexes);
$this->assertEquals([
'type' => 'unique',
'columns' => ['Number'],
], $indexes['IndexUnique']);
$this->assertArrayHasKey('IndexNormal', $indexes);
$this->assertEquals([
'type' => 'index',
'columns' => ['Title'],
], $indexes['IndexNormal']);
}
}

View File

@ -0,0 +1,28 @@
<?php
namespace SilverStripe\ORM\Tests\DataObjectSchemaTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\DataObject;
class AllIndexes extends DataObject implements TestOnly
{
private static $table_name = 'DataObjectSchemaTest_AllIndexes';
private static $db = [
'Title' => 'Varchar',
'Content' => 'Varchar',
'Number' => 'Int',
];
private static $indexes = [
'Content' => true,
'IndexCols' => ['Title', 'Content'],
'IndexUnique' => [
'type' => 'unique',
'columns' => ['Number'],
],
'IndexNormal' => [
'columns' => ['Title'],
],
];
}

View File

@ -11,22 +11,28 @@ class TestObject extends DataObject implements TestOnly
private static $table_name = 'FulltextFilterTest_DataObject';
private static $db = array(
"ColumnA" => "Varchar(255)",
"ColumnB" => "HTMLText",
"ColumnC" => "Varchar(255)",
"ColumnD" => "HTMLText",
"ColumnE" => 'Varchar(255)'
);
private static $db = [
'ColumnA' => 'Varchar(255)',
'ColumnB' => 'HTMLText',
'ColumnC' => 'Varchar(255)',
'ColumnD' => 'HTMLText',
'ColumnE' => 'Varchar(255)',
];
private static $indexes = array(
'SearchFields' => array(
'SearchFields' => [
'type' => 'fulltext',
'name' => 'SearchFields',
'value' => '"ColumnA", "ColumnB"',
),
'OtherSearchFields' => 'fulltext ("ColumnC", "ColumnD")',
'SingleIndex' => 'fulltext ("ColumnE")'
'columns' => ['ColumnA', 'ColumnB'],
],
'OtherSearchFields' => [
'type' => 'fulltext',
'columns' => ['ColumnC', 'ColumnD'],
],
'SingleIndex' => [
'type' => 'fulltext',
'columns' => ['ColumnE'],
],
);
private static $create_table_options = array(

View File

@ -6,6 +6,7 @@ use SilverStripe\Assets\File;
use SilverStripe\ORM\Connect\MySQLSchemaManager;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Search\FulltextSearchable;
class FulltextSearchableTest extends SapphireTest
@ -49,4 +50,13 @@ class FulltextSearchableTest extends SapphireTest
File::remove_extension(FulltextSearchable::class);
$this->assertFalse(File::has_extension(FulltextSearchable::class));
}
public function testIndexesAdded()
{
$indexes = DataObject::getSchema()->databaseIndexes(File::class);
$this->assertArrayHasKey('SearchFields', $indexes);
$this->assertCount(2, $indexes['SearchFields']['columns']);
$this->assertContains('Name', $indexes['SearchFields']['columns']);
$this->assertContains('Title', $indexes['SearchFields']['columns']);
}
}