Permalink
Browse files

FIXED: Generation of tables with fulltext indexes now no longer incor…

…rectly 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.
  • Loading branch information...
1 parent bb8a92c commit 30e15d11a7f98c420c164efea08e74394a450e93 @tractorcow tractorcow committed Sep 11, 2012
View
@@ -421,14 +421,6 @@ function requireIndex($table, $index, $spec) {
}
}
- //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
@@ -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,
@@ -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 @@ static function enable($searchableClasses = array('SiteTree', 'File')) {
* 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();
@@ -4,33 +4,141 @@ 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 {
static $db = array(
'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"'
+ )
+ );
}
@@ -12,12 +12,12 @@ function setUp() {
$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 @@ function testEnableWithCustomClasses() {
$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'));
}

0 comments on commit 30e15d1

Please sign in to comment.