FIXED: Generation of tables with fulltext indexes now no longer incorrectly thinks that all fulltext indexes have changed.

ADDED: Test cases correctly checking for changes (and no changes) to the data model for both fields and indexes.
FIXED: References to indexes throughough the code that probably should have quoted field names. This prevents a lot of 'spam' during dev build. This includes an updated FulltextSearchable test case.
This commit is contained in:
Damian Mooyman 2012-09-11 12:53:08 +12:00
parent bb8a92cde7
commit 30e15d11a7
5 changed files with 120 additions and 20 deletions

View File

@ -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");

View File

@ -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,

View File

@ -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();

View File

@ -4,27 +4,117 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
protected $extraDataObjects = array(
'DataObjectSchemaGenerationTest_DO',
);
/**
* 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();
$db->endSchemaUpdate();
// Test table within this database
$db->beginSchemaUpdate();
$obj2 = new DataObjectSchemaGenerationTest_DO();
$obj2->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
// Verify that it doesn't need to be recreated
$db->beginSchemaUpdate();
$obj = new DataObjectSchemaGenerationTest_DO();
$obj->requireTable();
$db->endSchemaUpdate();
// Let's insert a new field here
DataObjectSchemaGenerationTest_DO::$db['SecretField'] = 'Varchar(100)';
// Test table within this database
$db->beginSchemaUpdate();
$obj2 = new DataObjectSchemaGenerationTest_DO();
$obj2->requireTable();
$needsUpdating = $db->doesSchemaNeedUpdating();
$db->cancelSchemaUpdate();
$this->assertTrue($needsUpdating);
}
/**
* Check that indexes on a newly generated class do not subsequently request modification
*/
function testIndexesDontRerequestChanges() {
$db = DB::getConn();
DB::quiet();
// enable fulltext option on this table
Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'create_table_options', array('MySQLDatabase' => 'ENGINE=MyISAM'));
// 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();
$db->endSchemaUpdate();
// Test table within this database
$db->beginSchemaUpdate();
$obj2 = new DataObjectSchemaGenerationTest_IndexDO();
$obj2->requireTable();
$needsUpdating = $db->doesSchemaNeedUpdating();
$db->cancelSchemaUpdate();
$this->assertFalse($needsUpdating);
}
/**
* Check that updates to a dataobject's indexes are reflected in DDL
*/
function testIndexesRerequestChanges() {
$db = DB::getConn();
DB::quiet();
// enable fulltext option on this table
Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'create_table_options', array('MySQLDatabase' => 'ENGINE=MyISAM'));
// 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();
$db->endSchemaUpdate();
// Let's insert a new field here
DataObjectSchemaGenerationTest_IndexDO::$indexes['SearchFields']['value'] = '"Title"';
// Test table within this database
$db->beginSchemaUpdate();
$obj2 = new DataObjectSchemaGenerationTest_IndexDO();
$obj2->requireTable();
$needsUpdating = $db->doesSchemaNeedUpdating();
$db->cancelSchemaUpdate();
$this->assertTrue($needsUpdating);
}
}
class DataObjectSchemaGenerationTest_DO extends DataObject implements TestOnly {
@ -32,5 +122,23 @@ 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(
// Space between 'unique' and '("Name")' is critical. @todo - Robustify?
'NameIndex' => 'unique ("Title")',
'SearchFields' => array(
'type' => 'fulltext',
'name' => 'SearchFields',
'value' => '"Title","Content"'
)
);
}

View File

@ -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'));
}