diff --git a/model/Database.php b/model/Database.php index 7e32cd114..a33e8a6c8 100644 --- a/model/Database.php +++ b/model/Database.php @@ -421,14 +421,6 @@ abstract class SS_Database { } } - //We need to include the name of the fulltext index here so we can trigger a rebuild - //if either the name or the columns have changed. - if(is_array($spec) && isset($spec['type'])){ - if($spec['type']=='fulltext'){ - $array_spec="({$spec['name']},{$spec['value']})"; - } - } - if($newTable || !isset($this->indexList[$table][$index_alt])) { $this->transCreateIndex($table, $index, $spec); $this->alterationMessage("Index $table.$index: created as " . DB::getConn()->convertIndexSpec($spec),"created"); diff --git a/model/Versioned.php b/model/Versioned.php index b8efd4d87..c3b5aa472 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -70,7 +70,7 @@ class Versioned extends DataExtension { * @var array $indexes_for_versions_table */ static $indexes_for_versions_table = array( - 'RecordID_Version' => '(RecordID,Version)', + 'RecordID_Version' => '("RecordID","Version")', 'RecordID' => true, 'Version' => true, 'AuthorID' => true, diff --git a/search/FulltextSearchable.php b/search/FulltextSearchable.php index 006ac028b..2ceb3389e 100644 --- a/search/FulltextSearchable.php +++ b/search/FulltextSearchable.php @@ -43,8 +43,8 @@ class FulltextSearchable extends DataExtension { */ static function enable($searchableClasses = array('SiteTree', 'File')) { $defaultColumns = array( - 'SiteTree' => 'Title,MenuTitle,Content,MetaTitle,MetaDescription,MetaKeywords', - 'File' => 'Filename,Title,Content' + 'SiteTree' => '"Title","MenuTitle","Content","MetaTitle","MetaDescription","MetaKeywords"', + 'File' => '"Filename","Title","Content"' ); if(!is_array($searchableClasses)) $searchableClasses = array($searchableClasses); @@ -69,7 +69,7 @@ class FulltextSearchable extends DataExtension { * that can be searched on. Used for generation of the database index defintions. */ function __construct($searchFields = array()) { - if(is_array($searchFields)) $this->searchFields = implode(',', $searchFields); + if(is_array($searchFields)) $this->searchFields = '"'.implode('","', $searchFields).'"'; else $this->searchFields = $searchFields; parent::__construct(); diff --git a/tests/model/DataObjectSchemaGenerationTest.php b/tests/model/DataObjectSchemaGenerationTest.php index 2abfa55cc..aef5facc5 100644 --- a/tests/model/DataObjectSchemaGenerationTest.php +++ b/tests/model/DataObjectSchemaGenerationTest.php @@ -3,28 +3,117 @@ class DataObjectSchemaGenerationTest extends SapphireTest { protected $extraDataObjects = array( 'DataObjectSchemaGenerationTest_DO', + 'DataObjectSchemaGenerationTest_IndexDO' ); + function setUpOnce() { + + // enable fulltext option on this table + Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'create_table_options', array('MySQLDatabase' => 'ENGINE=MyISAM')); + + parent::setUpOnce(); + } + /** * Check that once a schema has been generated, then it doesn't need any more updating */ function testFieldsDontRerequestChanges() { $db = DB::getConn(); DB::quiet(); - // Table will have been initially created by the $extraDataObjects setting - // Verify that it doesn't need to be recreated $db->beginSchemaUpdate(); $obj = new DataObjectSchemaGenerationTest_DO(); $obj->requireTable(); $needsUpdating = $db->doesSchemaNeedUpdating(); $db->cancelSchemaUpdate(); - $this->assertFalse($needsUpdating); } + + /** + * Check that updates to a class fields are reflected in the database + */ + function testFieldsRequestChanges() { + $db = DB::getConn(); + DB::quiet(); + + // Table will have been initially created by the $extraDataObjects setting + + // Let's insert a new field here + $oldDB = DataObjectSchemaGenerationTest_DO::$db; + DataObjectSchemaGenerationTest_DO::$db['SecretField'] = 'Varchar(100)'; + + // Verify that the above extra field triggered a schema update + $db->beginSchemaUpdate(); + $obj = new DataObjectSchemaGenerationTest_DO(); + $obj->requireTable(); + $needsUpdating = $db->doesSchemaNeedUpdating(); + $db->cancelSchemaUpdate(); + $this->assertTrue($needsUpdating); + + // Restore db configuration + DataObjectSchemaGenerationTest_DO::$db = $oldDB; + } + + /** + * Check that indexes on a newly generated class do not subsequently request modification + */ + function testIndexesDontRerequestChanges() { + $db = DB::getConn(); + DB::quiet(); + + // Table will have been initially created by the $extraDataObjects setting + + // Verify that it doesn't need to be recreated + $db->beginSchemaUpdate(); + $obj = new DataObjectSchemaGenerationTest_IndexDO(); + $obj->requireTable(); + $needsUpdating = $db->doesSchemaNeedUpdating(); + $db->cancelSchemaUpdate(); + $this->assertFalse($needsUpdating); + + // Test with alternate index format, although these indexes are the same + $oldIndexes = DataObjectSchemaGenerationTest_IndexDO::$indexes; + DataObjectSchemaGenerationTest_IndexDO::$indexes = DataObjectSchemaGenerationTest_IndexDO::$indexes_alt; + + // Verify that it still doesn't need to be recreated + $db->beginSchemaUpdate(); + $obj2 = new DataObjectSchemaGenerationTest_IndexDO(); + $obj2->requireTable(); + $needsUpdating = $db->doesSchemaNeedUpdating(); + $db->cancelSchemaUpdate(); + $this->assertFalse($needsUpdating); + + // Restore old index format + DataObjectSchemaGenerationTest_IndexDO::$indexes = $oldIndexes; + } + + /** + * Check that updates to a dataobject's indexes are reflected in DDL + */ + function testIndexesRerequestChanges() { + $db = DB::getConn(); + DB::quiet(); + + // Table will have been initially created by the $extraDataObjects setting + + // Update the SearchFields index here + $oldIndexes = DataObjectSchemaGenerationTest_IndexDO::$indexes; + DataObjectSchemaGenerationTest_IndexDO::$indexes['SearchFields']['value'] = '"Title"'; + + // Verify that the above index change triggered a schema update + $db->beginSchemaUpdate(); + $obj = new DataObjectSchemaGenerationTest_IndexDO(); + $obj->requireTable(); + $needsUpdating = $db->doesSchemaNeedUpdating(); + $db->cancelSchemaUpdate(); + $this->assertTrue($needsUpdating); + + // Restore old indexes + DataObjectSchemaGenerationTest_IndexDO::$indexes = $oldIndexes; + } } class DataObjectSchemaGenerationTest_DO extends DataObject implements TestOnly { @@ -32,5 +121,30 @@ class DataObjectSchemaGenerationTest_DO extends DataObject implements TestOnly { 'Enum1' => 'Enum("A, B, C, D","")', 'Enum2' => 'Enum("A, B, C, D","A")', ); - } + + +class DataObjectSchemaGenerationTest_IndexDO extends DataObjectSchemaGenerationTest_DO implements TestOnly { + static $db = array( + 'Title' => 'Varchar(255)', + 'Content' => 'Text' + ); + + static $indexes = array( + 'NameIndex' => 'unique ("Title")', + 'SearchFields' => array( + 'type' => 'fulltext', + 'name' => 'SearchFields', + 'value' => '"Title","Content"' + ) + ); + + static $indexes_alt = array( + 'NameIndex' => array( + 'type' => 'unique', + 'name' => 'NameIndex', + 'value' => '"Title"' + ), + 'SearchFields' => 'fulltext ("Title","Content")' + ); +} \ No newline at end of file diff --git a/tests/search/FulltextSearchableTest.php b/tests/search/FulltextSearchableTest.php index 08075d343..2d327eaa1 100644 --- a/tests/search/FulltextSearchableTest.php +++ b/tests/search/FulltextSearchableTest.php @@ -12,12 +12,12 @@ class FulltextSearchableTest extends SapphireTest { $this->orig['File_searchable'] = Object::has_extension('File', 'FulltextSearchable'); // TODO This shouldn't need all arguments included - Object::remove_extension('File', 'FulltextSearchable(\'Filename,Title,Content\')'); + Object::remove_extension('File', 'FulltextSearchable(\'"Filename","Title","Content"\')'); } function tearDown() { // TODO This shouldn't need all arguments included - if($this->orig['File_searchable']) Object::add_extension('File', 'FulltextSearchable(\'Filename,Title,Content\')'); + if($this->orig['File_searchable']) Object::add_extension('File', 'FulltextSearchable(\'"Filename","Title","Content"\')'); parent::tearDown(); } @@ -32,7 +32,7 @@ class FulltextSearchableTest extends SapphireTest { $this->assertTrue(Object::has_extension('File', 'FulltextSearchable')); // TODO This shouldn't need all arguments included - Object::remove_extension('File', 'FulltextSearchable(\'Filename,Title,Content\')'); + Object::remove_extension('File', 'FulltextSearchable(\'"Filename","Title","Content"\')'); $this->assertFalse(Object::has_extension('File', 'FulltextSearchable')); }