From c9c43906191511bce8ef6f4dc8b84600eebaebf2 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 23 Jun 2017 17:40:06 +1200 Subject: [PATCH 1/3] NEW Ensure polymorphic has_one fields are indexed * Add tests for config based indexing on composite DBFields * Allow fields to have "indexed" option passed via field spec --- src/ORM/DataObjectSchema.php | 65 ++++++++++--- src/ORM/FieldType/DBClassName.php | 9 +- src/ORM/FieldType/DBComposite.php | 7 ++ src/ORM/FieldType/DBEnum.php | 5 +- src/ORM/FieldType/DBField.php | 91 ++++++++++++++++++- src/ORM/FieldType/DBForeignKey.php | 2 + src/ORM/FieldType/DBPolymorphicForeignKey.php | 3 +- src/ORM/FieldType/DBString.php | 45 ++++----- tests/php/ORM/DataObjectSchemaTest.php | 58 +++++++++++- .../DataObjectSchemaTest/HasComposites.php | 18 ++++ .../HasIndexesInFieldSpecs.php | 16 ++++ 11 files changed, 269 insertions(+), 50 deletions(-) create mode 100644 tests/php/ORM/DataObjectSchemaTest/HasComposites.php create mode 100644 tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index 236630fae..a71856b66 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -43,6 +43,13 @@ class DataObjectSchema */ protected $databaseIndexes = []; + /** + * Fields that should be indexed, by class name + * + * @var array + */ + protected $defaultDatabaseIndexes = []; + /** * Cache of composite database field * @@ -65,6 +72,7 @@ class DataObjectSchema $this->tableNames = []; $this->databaseFields = []; $this->databaseIndexes = []; + $this->defaultDatabaseIndexes = []; $this->compositeFields = []; } @@ -193,7 +201,6 @@ class DataObjectSchema foreach ($classes as $tableClass) { // Find all fields on this class $fields = $this->databaseFields($tableClass, false); - // Merge with composite fields if (!$dbOnly) { $compositeFields = $this->compositeFields($tableClass, false); @@ -486,30 +493,59 @@ class DataObjectSchema } /** - * Cache all indexes for the given class. - * Will do nothing if already cached + * 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; + if (!array_key_exists($class, $this->databaseIndexes)) { + $this->databaseIndexes[$class] = array_merge( + $this->cacheDefaultDatabaseIndexes($class), + $this->buildCustomDatabaseIndexes($class) + ); } - $indexes = []; + } - // look for indexable field types - foreach ($this->databaseFields($class, false) as $field => $type) { - /** @skipUpgrade */ - if ($type === 'ForeignKey' || $type === 'DBClassName') { - $indexes[$field] = [ + /** + * Get "default" database indexable field types + * + * @param string $class + * @return array + */ + protected function cacheDefaultDatabaseIndexes($class) + { + $indexes = []; + if (array_key_exists($class, $this->defaultDatabaseIndexes)) { + return $this->defaultDatabaseIndexes[$class]; + } + $this->defaultDatabaseIndexes[$class] = []; + + $fieldSpecs = $this->fieldSpecs($class, self::UNINHERITED); + foreach ($fieldSpecs as $field => $spec) { + /** @var DBField $fieldObj */ + $fieldObj = Injector::inst()->create($spec, $field); + if ($fieldObj->getIndexed()) { + $this->defaultDatabaseIndexes[$class][$field] = [ 'type' => 'index', - 'columns' => [$field], + 'columns' => $fieldObj->getIndexSpecs(), ]; } } + return $this->defaultDatabaseIndexes[$class]; + } - // look for custom indexes declared on the class + /** + * Look for custom indexes declared on the class + * + * @param string $class + * @return array + * @throws InvalidArgumentException If an index already exists on the class + * @throws InvalidArgumentException If a custom index format is not valid + */ + protected function buildCustomDatabaseIndexes($class) + { + $indexes = []; $classIndexes = Config::inst()->get($class, 'indexes', Config::UNINHERITED) ?: []; foreach ($classIndexes as $indexName => $indexSpec) { if (array_key_exists($indexName, $indexes)) { @@ -546,8 +582,7 @@ class DataObjectSchema } $indexes[$indexName] = $indexSpec; } - - $this->databaseIndexes[$class] = $indexes; + return $indexes; } /** diff --git a/src/ORM/FieldType/DBClassName.php b/src/ORM/FieldType/DBClassName.php index 276a856f2..09dcb4d10 100644 --- a/src/ORM/FieldType/DBClassName.php +++ b/src/ORM/FieldType/DBClassName.php @@ -37,6 +37,8 @@ class DBClassName extends DBEnum */ protected static $classname_cache = array(); + private static $indexed = true; + /** * Clear all cached classname specs. It's necessary to clear all cached subclassed names * for any classes if a new class manifest is generated. @@ -49,13 +51,14 @@ class DBClassName extends DBEnum /** * Create a new DBClassName field * - * @param string $name Name of field + * @param string $name Name of field * @param string|null $baseClass Optional base class to limit selections + * @param array $options Optional parameters for this DBField instance */ - public function __construct($name = null, $baseClass = null) + public function __construct($name = null, $baseClass = null, $options = []) { $this->setBaseClass($baseClass); - parent::__construct($name); + parent::__construct($name, null, null, $options); } /** diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index 8ad958611..a7ba4ae0c 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -293,4 +293,11 @@ abstract class DBComposite extends DBField return parent::castingHelper($field); } + + public function getIndexSpecs() + { + return array_map(function ($name) { + return $this->getName() . $name; + }, array_keys((array) static::config()->get('composite_db'))); + } } diff --git a/src/ORM/FieldType/DBEnum.php b/src/ORM/FieldType/DBEnum.php index 59f85fc42..fc7802de7 100644 --- a/src/ORM/FieldType/DBEnum.php +++ b/src/ORM/FieldType/DBEnum.php @@ -51,8 +51,9 @@ class DBEnum extends DBString * @param string $name * @param string|array $enum A string containing a comma separated list of options or an array of Vals. * @param string $default The default option, which is either NULL or one of the items in the enumeration. + * @param array $options Optional parameters for this DB field */ - public function __construct($name = null, $enum = null, $default = null) + public function __construct($name = null, $enum = null, $default = null, $options = []) { if ($enum) { $this->setEnum($enum); @@ -73,7 +74,7 @@ class DBEnum extends DBString } } - parent::__construct($name); + parent::__construct($name, $options); } /** diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index c43253b61..4abc05d5e 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -74,6 +74,13 @@ abstract class DBField extends ViewableData */ protected $arrayValue; + /** + * Optional parameters for this field + * + * @var array + */ + protected $options = []; + /** * The escape type for this field when inserted into a template - either "xml" or "raw". * @@ -90,6 +97,16 @@ abstract class DBField extends ViewableData */ private static $default_search_filter_class = 'PartialMatchFilter'; + /** + * Whether this field type should be indexed by default. Can be overridden by {@link setOptions()} + * with "indexed" => true or via a DBField constructor, e.g. "MyField" => "Varchar(255, ['indexed' => false])". + * Note: if doing this, be aware of the existing field constructor arguments, like the length above. + * + * @var bool + * @config + */ + private static $indexed = false; + private static $casting = array( 'ATT' => 'HTMLFragment', 'CDATA' => 'HTMLFragment', @@ -110,10 +127,24 @@ abstract class DBField extends ViewableData */ protected $defaultVal; - public function __construct($name = null) + /** + * Provide the DBField name and an array of options, e.g. ['indexed' => true], or ['nullifyEmpty' => false] + * + * @param string $name + * @param array $options + * @throws InvalidArgumentException If $options was passed by not an array + */ + public function __construct($name = null, $options = []) { $this->name = $name; + if ($options) { + if (!is_array($options)) { + throw new \InvalidArgumentException("Invalid options $options"); + } + $this->setOptions($options); + } + parent::__construct(); } @@ -224,6 +255,54 @@ abstract class DBField extends ViewableData return $this; } + /** + * Update the optional parameters for this field + * + * @param array $options Array of options + * @return $this + */ + public function setOptions(array $options = []) + { + $this->options = $options; + return $this; + } + + /** + * Get optional parameters for this field + * + * @return array + */ + public function getOptions() + { + return $this->options; + } + + /** + * Set whether this DBField instance should be indexed + * + * @param bool $indexed + * @return $this + */ + public function setIndexed($indexed) + { + $this->options['indexed'] = (bool) $indexed; + return $this; + } + + /** + * Get whether this DBField should be indexed + * + * @return bool + */ + public function getIndexed() + { + if (array_key_exists('indexed', $this->options)) { + return (bool) $this->options['indexed']; + } + + // Default to global configuration per DBField instance + return (bool) static::config()->get('indexed'); + } /** * Determines if the field has a value which is not considered to be 'null' @@ -549,4 +628,14 @@ DBG; { return $this->RAW(); } + + /** + * Get the field(s) that should be used if this field is indexed + * + * @return array + */ + public function getIndexSpecs() + { + return [$this->getName()]; + } } diff --git a/src/ORM/FieldType/DBForeignKey.php b/src/ORM/FieldType/DBForeignKey.php index 7c3d6beb5..622db5f03 100644 --- a/src/ORM/FieldType/DBForeignKey.php +++ b/src/ORM/FieldType/DBForeignKey.php @@ -29,6 +29,8 @@ class DBForeignKey extends DBInt */ protected $object; + private static $indexed = true; + private static $default_search_filter_class = 'ExactMatchFilter'; public function __construct($name, $object = null) diff --git a/src/ORM/FieldType/DBPolymorphicForeignKey.php b/src/ORM/FieldType/DBPolymorphicForeignKey.php index aea9344b4..85bdd8f1c 100644 --- a/src/ORM/FieldType/DBPolymorphicForeignKey.php +++ b/src/ORM/FieldType/DBPolymorphicForeignKey.php @@ -9,10 +9,11 @@ use SilverStripe\ORM\DataObject; */ class DBPolymorphicForeignKey extends DBComposite { + private static $indexed = true; private static $composite_db = array( 'ID' => 'Int', - 'Class' => "DBClassName('SilverStripe\\ORM\\DataObject')" + 'Class' => "DBClassName('" . DataObject::class . "', ['indexed' => false])" ); public function scaffoldFormField($title = null, $params = null) diff --git a/src/ORM/FieldType/DBString.php b/src/ORM/FieldType/DBString.php index de5e4730b..3732c0952 100644 --- a/src/ORM/FieldType/DBString.php +++ b/src/ORM/FieldType/DBString.php @@ -7,12 +7,6 @@ namespace SilverStripe\ORM\FieldType; */ abstract class DBString extends DBField { - - /** - * @var boolean - */ - protected $nullifyEmpty = true; - /** * @var array */ @@ -26,22 +20,14 @@ abstract class DBString extends DBField ); /** - * Construct a string type field with a set of optional parameters. + * Set the default value for "nullify empty" * - * @param string $name string The name of the field - * @param array $options array An array of options e.g. array('nullifyEmpty'=>false). See - * {@link StringField::setOptions()} for information on the available options + * {@inheritDoc} */ - public function __construct($name = null, $options = array()) + public function __construct($name = null, $options = []) { - if ($options) { - if (!is_array($options)) { - throw new \InvalidArgumentException("Invalid options $options"); - } - $this->setOptions($options); - } - - parent::__construct($name); + $this->options['nullifyEmpty'] = true; + parent::__construct($name, $options); } /** @@ -56,14 +42,17 @@ abstract class DBString extends DBField * * @return $this */ - public function setOptions(array $options = array()) + public function setOptions(array $options = []) { - if (array_key_exists("nullifyEmpty", $options)) { - $this->nullifyEmpty = $options["nullifyEmpty"] ? true : false; + parent::setOptions($options); + + if (array_key_exists('nullifyEmpty', $options)) { + $this->options['nullifyEmpty'] = (bool) $options['nullifyEmpty']; } - if (array_key_exists("default", $options)) { - $this->setDefaultValue($options["default"]); + if (array_key_exists('default', $options)) { + $this->setDefaultValue($options['default']); } + return $this; } @@ -72,10 +61,12 @@ abstract class DBString extends DBField * them to null. * * @param $value boolean True if empty strings are to be converted to null + * @return $this */ public function setNullifyEmpty($value) { - $this->nullifyEmpty = ($value ? true : false); + $this->options['nullifyEmpty'] = (bool) $value; + return $this; } /** @@ -86,7 +77,7 @@ abstract class DBString extends DBField */ public function getNullifyEmpty() { - return $this->nullifyEmpty; + return $this->options['nullifyEmpty']; } /** @@ -108,7 +99,7 @@ abstract class DBString extends DBField } // Return "empty" value - if ($this->nullifyEmpty || $value === null) { + if ($this->options['nullifyEmpty'] || $value === null) { return null; } return ''; diff --git a/tests/php/ORM/DataObjectSchemaTest.php b/tests/php/ORM/DataObjectSchemaTest.php index 242094bcc..02529dfa9 100644 --- a/tests/php/ORM/DataObjectSchemaTest.php +++ b/tests/php/ORM/DataObjectSchemaTest.php @@ -3,8 +3,10 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\Core\ClassInfo; -use SilverStripe\ORM\DataObject; +use SilverStripe\Core\Config\Config; +use SilverStripe\ORM\FieldType\DBMoney; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\Tests\DataObjectSchemaTest\AllIndexes; use SilverStripe\ORM\Tests\DataObjectSchemaTest\BaseClass; @@ -12,7 +14,9 @@ use SilverStripe\ORM\Tests\DataObjectSchemaTest\BaseDataClass; use SilverStripe\ORM\Tests\DataObjectSchemaTest\ChildClass; use SilverStripe\ORM\Tests\DataObjectSchemaTest\DefaultTableName; use SilverStripe\ORM\Tests\DataObjectSchemaTest\GrandChildClass; +use SilverStripe\ORM\Tests\DataObjectSchemaTest\HasComposites; use SilverStripe\ORM\Tests\DataObjectSchemaTest\HasFields; +use SilverStripe\ORM\Tests\DataObjectSchemaTest\HasIndexesInFieldSpecs; use SilverStripe\ORM\Tests\DataObjectSchemaTest\NoFields; use SilverStripe\ORM\Tests\DataObjectSchemaTest\WithCustomTable; use SilverStripe\ORM\Tests\DataObjectSchemaTest\WithRelation; @@ -271,4 +275,56 @@ class DataObjectSchemaTest extends SapphireTest 'columns' => ['Title'], ], $indexes['IndexNormal']); } + + public function testCompositeDatabaseFieldIndexes() + { + $indexes = DataObject::getSchema()->databaseIndexes(HasComposites::class); + $this->assertCount(3, $indexes); + $this->assertArrayHasKey('RegularHasOneID', $indexes); + $this->assertEquals([ + 'type' => 'index', + 'columns' => ['RegularHasOneID'] + ], $indexes['RegularHasOneID']); + + $this->assertArrayHasKey('Polymorpheus', $indexes); + $this->assertEquals([ + 'type' => 'index', + 'columns' => ['PolymorpheusID', 'PolymorpheusClass'] + ], $indexes['Polymorpheus']); + + // Check that DBPolymorphicForeignKey's "Class" is not indexed on its own + $this->assertArrayNotHasKey('PolymorpheusClass', $indexes); + } + + public function testCompositeFieldsCanBeIndexedByDefaultConfiguration() + { + Config::modify()->set(DBMoney::class, 'indexed', true); + $indexes = DataObject::getSchema()->databaseIndexes(HasComposites::class); + $this->assertCount(4, $indexes); + $this->assertArrayHasKey('Amount', $indexes); + $this->assertEquals([ + 'type' => 'index', + 'columns' => ['AmountCurrency', 'AmountAmount'] + ], $indexes['Amount']); + } + + public function testFieldsCanBeIndexedFromFieldSpecs() + { + $indexes = DataObject::getSchema()->databaseIndexes(HasIndexesInFieldSpecs::class); + + $this->assertCount(3, $indexes); + $this->assertArrayHasKey('ClassName', $indexes); + + $this->assertArrayHasKey('IndexedTitle', $indexes); + $this->assertEquals([ + 'type' => 'index', + 'columns' => ['IndexedTitle'] + ], $indexes['IndexedTitle']); + + $this->assertArrayHasKey('IndexedMoney', $indexes); + $this->assertEquals([ + 'type' => 'index', + 'columns' => ['IndexedMoneyCurrency', 'IndexedMoneyAmount'] + ], $indexes['IndexedMoney']); + } } diff --git a/tests/php/ORM/DataObjectSchemaTest/HasComposites.php b/tests/php/ORM/DataObjectSchemaTest/HasComposites.php new file mode 100644 index 000000000..06a030acf --- /dev/null +++ b/tests/php/ORM/DataObjectSchemaTest/HasComposites.php @@ -0,0 +1,18 @@ + 'Money' + ]; + + private static $has_one = [ + 'RegularHasOne' => ChildClass::class, + 'Polymorpheus' => DataObject::class + ]; +} diff --git a/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php b/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php new file mode 100644 index 000000000..914faa009 --- /dev/null +++ b/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php @@ -0,0 +1,16 @@ + 'Varchar', + 'IndexedTitle' => 'Varchar(255, ["indexed" => true])', + 'NormalMoney' => 'Money', + 'IndexedMoney' => 'Money(["indexed" => true])' + ]; +} From bd5782adcaf46e4a7281b778504a38e4dc6badf3 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 4 Jul 2017 21:58:07 +1200 Subject: [PATCH 2/3] NEW Allow index type to be configured per DBField instance --- src/ORM/DataObjectSchema.php | 4 +- src/ORM/FieldType/DBClassName.php | 2 +- src/ORM/FieldType/DBField.php | 59 +++++++++---------- src/ORM/FieldType/DBForeignKey.php | 2 +- src/ORM/FieldType/DBIndexable.php | 50 ++++++++++++++++ src/ORM/FieldType/DBPolymorphicForeignKey.php | 4 +- tests/php/ORM/DataObjectSchemaTest.php | 20 ++++++- .../ORM/DataObjectSchemaTest/AllIndexes.php | 3 +- .../HasIndexesInFieldSpecs.php | 4 +- 9 files changed, 106 insertions(+), 42 deletions(-) create mode 100644 src/ORM/FieldType/DBIndexable.php diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index a71856b66..1455d4136 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -525,9 +525,9 @@ class DataObjectSchema foreach ($fieldSpecs as $field => $spec) { /** @var DBField $fieldObj */ $fieldObj = Injector::inst()->create($spec, $field); - if ($fieldObj->getIndexed()) { + if ($type = $fieldObj->getIndexType()) { $this->defaultDatabaseIndexes[$class][$field] = [ - 'type' => 'index', + 'type' => $type, 'columns' => $fieldObj->getIndexSpecs(), ]; } diff --git a/src/ORM/FieldType/DBClassName.php b/src/ORM/FieldType/DBClassName.php index 09dcb4d10..94bb2bffa 100644 --- a/src/ORM/FieldType/DBClassName.php +++ b/src/ORM/FieldType/DBClassName.php @@ -37,7 +37,7 @@ class DBClassName extends DBEnum */ protected static $classname_cache = array(); - private static $indexed = true; + private static $index = true; /** * Clear all cached classname specs. It's necessary to clear all cached subclassed names diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index 4abc05d5e..afeab877e 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -43,7 +43,7 @@ use SilverStripe\View\ViewableData; * * @todo remove MySQL specific code from subclasses */ -abstract class DBField extends ViewableData +abstract class DBField extends ViewableData implements DBIndexable { /** @@ -98,14 +98,13 @@ abstract class DBField extends ViewableData private static $default_search_filter_class = 'PartialMatchFilter'; /** - * Whether this field type should be indexed by default. Can be overridden by {@link setOptions()} - * with "indexed" => true or via a DBField constructor, e.g. "MyField" => "Varchar(255, ['indexed' => false])". - * Note: if doing this, be aware of the existing field constructor arguments, like the length above. + * The type of index to use for this field. Can either be a string (one of the DBIndexable type options) or a + * boolean. When a boolean is given, false will not index the field, and true will use the default index type. * - * @var bool + * @var string|bool * @config */ - private static $indexed = false; + private static $index = false; private static $casting = array( 'ATT' => 'HTMLFragment', @@ -128,7 +127,7 @@ abstract class DBField extends ViewableData protected $defaultVal; /** - * Provide the DBField name and an array of options, e.g. ['indexed' => true], or ['nullifyEmpty' => false] + * Provide the DBField name and an array of options, e.g. ['index' => true], or ['nullifyEmpty' => false] * * @param string $name * @param array $options @@ -277,31 +276,36 @@ abstract class DBField extends ViewableData return $this->options; } - /** - * Set whether this DBField instance should be indexed - * - * @param bool $indexed - * @return $this - */ - public function setIndexed($indexed) + public function setIndexType($type) { - $this->options['indexed'] = (bool) $indexed; + if (!is_bool($type) + && !in_array($type, [DBIndexable::TYPE_INDEX, DBIndexable::TYPE_UNIQUE, DBIndexable::TYPE_FULLTEXT]) + ) { + throw new \InvalidArgumentException( + "{$type} is not a valid index type or boolean. Please see DBIndexable." + ); + } + + $this->options['index'] = $type; return $this; } - /** - * Get whether this DBField should be indexed - * - * @return bool - */ - public function getIndexed() + public function getIndexType() { - if (array_key_exists('indexed', $this->options)) { - return (bool) $this->options['indexed']; + if (array_key_exists('index', $this->options)) { + $type = $this->options['index']; + } else { + $type = static::config()->get('index'); } - // Default to global configuration per DBField instance - return (bool) static::config()->get('indexed'); + if (is_bool($type)) { + if (!$type) { + return false; + } + $type = DBIndexable::TYPE_DEFAULT; + } + + return $type; } /** @@ -629,11 +633,6 @@ DBG; return $this->RAW(); } - /** - * Get the field(s) that should be used if this field is indexed - * - * @return array - */ public function getIndexSpecs() { return [$this->getName()]; diff --git a/src/ORM/FieldType/DBForeignKey.php b/src/ORM/FieldType/DBForeignKey.php index 622db5f03..c01a945be 100644 --- a/src/ORM/FieldType/DBForeignKey.php +++ b/src/ORM/FieldType/DBForeignKey.php @@ -29,7 +29,7 @@ class DBForeignKey extends DBInt */ protected $object; - private static $indexed = true; + private static $index = true; private static $default_search_filter_class = 'ExactMatchFilter'; diff --git a/src/ORM/FieldType/DBIndexable.php b/src/ORM/FieldType/DBIndexable.php new file mode 100644 index 000000000..b710c012d --- /dev/null +++ b/src/ORM/FieldType/DBIndexable.php @@ -0,0 +1,50 @@ + 'Int', - 'Class' => "DBClassName('" . DataObject::class . "', ['indexed' => false])" + 'Class' => "DBClassName('" . DataObject::class . "', ['index' => false])" ); public function scaffoldFormField($title = null, $params = null) diff --git a/tests/php/ORM/DataObjectSchemaTest.php b/tests/php/ORM/DataObjectSchemaTest.php index 02529dfa9..483c1d9bf 100644 --- a/tests/php/ORM/DataObjectSchemaTest.php +++ b/tests/php/ORM/DataObjectSchemaTest.php @@ -4,8 +4,8 @@ namespace SilverStripe\ORM\Tests; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; -use SilverStripe\ORM\FieldType\DBMoney; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\FieldType\DBMoney; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\Tests\DataObjectSchemaTest\AllIndexes; @@ -298,8 +298,9 @@ class DataObjectSchemaTest extends SapphireTest public function testCompositeFieldsCanBeIndexedByDefaultConfiguration() { - Config::modify()->set(DBMoney::class, 'indexed', true); + Config::modify()->set(DBMoney::class, 'index', true); $indexes = DataObject::getSchema()->databaseIndexes(HasComposites::class); + $this->assertCount(4, $indexes); $this->assertArrayHasKey('Amount', $indexes); $this->assertEquals([ @@ -308,6 +309,19 @@ class DataObjectSchemaTest extends SapphireTest ], $indexes['Amount']); } + public function testIndexTypeIsConfigurable() + { + Config::modify()->set(DBMoney::class, 'index', 'unique'); + + $indexes = DataObject::getSchema()->databaseIndexes(HasComposites::class); + $this->assertCount(4, $indexes); + $this->assertArrayHasKey('Amount', $indexes); + $this->assertEquals([ + 'type' => 'unique', + 'columns' => ['AmountCurrency', 'AmountAmount'] + ], $indexes['Amount']); + } + public function testFieldsCanBeIndexedFromFieldSpecs() { $indexes = DataObject::getSchema()->databaseIndexes(HasIndexesInFieldSpecs::class); @@ -317,7 +331,7 @@ class DataObjectSchemaTest extends SapphireTest $this->assertArrayHasKey('IndexedTitle', $indexes); $this->assertEquals([ - 'type' => 'index', + 'type' => 'fulltext', 'columns' => ['IndexedTitle'] ], $indexes['IndexedTitle']); diff --git a/tests/php/ORM/DataObjectSchemaTest/AllIndexes.php b/tests/php/ORM/DataObjectSchemaTest/AllIndexes.php index 84bd58590..ae36d951a 100644 --- a/tests/php/ORM/DataObjectSchemaTest/AllIndexes.php +++ b/tests/php/ORM/DataObjectSchemaTest/AllIndexes.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\Tests\DataObjectSchemaTest; use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\FieldType\DBIndexable; class AllIndexes extends DataObject implements TestOnly { @@ -18,7 +19,7 @@ class AllIndexes extends DataObject implements TestOnly 'Content' => true, 'IndexCols' => ['Title', 'Content'], 'IndexUnique' => [ - 'type' => 'unique', + 'type' => DBIndexable::TYPE_UNIQUE, 'columns' => ['Number'], ], 'IndexNormal' => [ diff --git a/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php b/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php index 914faa009..dd6377583 100644 --- a/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php +++ b/tests/php/ORM/DataObjectSchemaTest/HasIndexesInFieldSpecs.php @@ -9,8 +9,8 @@ class HasIndexesInFieldSpecs extends DataObject implements TestOnly { private static $db = [ 'Normal' => 'Varchar', - 'IndexedTitle' => 'Varchar(255, ["indexed" => true])', + 'IndexedTitle' => 'Varchar(255, ["index" => "fulltext"])', 'NormalMoney' => 'Money', - 'IndexedMoney' => 'Money(["indexed" => true])' + 'IndexedMoney' => 'Money(["index" => true])' ]; } From fb18e441a72ad9cf1a3200361efc75e843ea3f39 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 18 Jul 2017 15:03:56 +1200 Subject: [PATCH 3/3] DBIndexable::getIndexSpecs is responsible for returning a DBFields full indexable spec --- src/ORM/DataObjectSchema.php | 7 ++----- src/ORM/FieldType/DBComposite.php | 13 ++++++++++--- src/ORM/FieldType/DBField.php | 7 ++++++- src/ORM/FieldType/DBIndexable.php | 9 ++++++++- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index 1455d4136..0944dd762 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -525,11 +525,8 @@ class DataObjectSchema foreach ($fieldSpecs as $field => $spec) { /** @var DBField $fieldObj */ $fieldObj = Injector::inst()->create($spec, $field); - if ($type = $fieldObj->getIndexType()) { - $this->defaultDatabaseIndexes[$class][$field] = [ - 'type' => $type, - 'columns' => $fieldObj->getIndexSpecs(), - ]; + if ($indexSpecs = $fieldObj->getIndexSpecs()) { + $this->defaultDatabaseIndexes[$class][$field] = $indexSpecs; } } return $this->defaultDatabaseIndexes[$class]; diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index a7ba4ae0c..21751ade5 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -296,8 +296,15 @@ abstract class DBComposite extends DBField public function getIndexSpecs() { - return array_map(function ($name) { - return $this->getName() . $name; - }, array_keys((array) static::config()->get('composite_db'))); + if ($type = $this->getIndexType()) { + $columns = array_map(function ($name) { + return $this->getName() . $name; + }, array_keys((array) static::config()->get('composite_db'))); + + return [ + 'type' => $type, + 'columns' => $columns, + ]; + } } } diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index afeab877e..8e692f1ff 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -635,6 +635,11 @@ DBG; public function getIndexSpecs() { - return [$this->getName()]; + if ($type = $this->getIndexType()) { + return [ + 'type' => $type, + 'columns' => [$this->getName()], + ]; + } } } diff --git a/src/ORM/FieldType/DBIndexable.php b/src/ORM/FieldType/DBIndexable.php index b710c012d..6c719677e 100644 --- a/src/ORM/FieldType/DBIndexable.php +++ b/src/ORM/FieldType/DBIndexable.php @@ -42,7 +42,14 @@ interface DBIndexable public function getIndexType(); /** - * Get the field(s) that should be used if this instance is indexed + * Returns the index specifications for the field instance, for example: + * + * + * [ + * 'type' => 'unique', + * 'columns' => ['FieldName'] + * ] + * * * @return array */