mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
BUGFIX: Fixed potential data corruption issue when you are changing the class of a SiteTree subclass between two subclasses that share a fieldname.
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@97594 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
parent
46b9ce6988
commit
58cf79731b
@ -2459,6 +2459,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
||||
|
||||
$baseClass = array_shift($tableClasses);
|
||||
|
||||
|
||||
// $collidingFields will keep a list fields that appear in mulitple places in the class
|
||||
// heirarchy for this table. They will be dealt with more explicitly in the SQL query
|
||||
// to ensure that junk data from other tables doesn't corrupt data objects
|
||||
$collidingFields = array();
|
||||
|
||||
// Build our intial query
|
||||
$query = new SQLQuery(array());
|
||||
$query->from("\"$baseClass\"");
|
||||
@ -2469,7 +2475,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
||||
if(!in_array($k, array('ClassName', 'LastEdited', 'Created')) && ClassInfo::classImplements($v, 'CompositeDBField')) {
|
||||
$this->dbObject($k)->addToQuery($query);
|
||||
} else {
|
||||
$query->select[] = "\"$baseClass\".\"$k\"";
|
||||
$query->select[$k] = "\"$baseClass\".\"$k\"";
|
||||
}
|
||||
}
|
||||
// Join all the tables
|
||||
@ -2482,7 +2488,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
||||
$compositeFields = self::composite_fields($tableClass, false);
|
||||
if($databaseFields) foreach($databaseFields as $k => $v) {
|
||||
if(!isset($compositeFields[$k])) {
|
||||
$query->select[] = "\"$tableClass\".\"$k\"";
|
||||
// Update $collidingFields if necessary
|
||||
if(isset($query->select[$k])) {
|
||||
if(!isset($collidingFields[$k])) $collidingFields[$k] = array($query->select[$k]);
|
||||
$collidingFields[$k][] = "\"$tableClass\".\"$k\"";
|
||||
|
||||
} else {
|
||||
$query->select[$k] = "\"$tableClass\".\"$k\"";
|
||||
}
|
||||
}
|
||||
}
|
||||
if($compositeFields) foreach($compositeFields as $k => $v) {
|
||||
@ -2491,6 +2504,26 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve colliding fields
|
||||
if($collidingFields) {
|
||||
foreach($collidingFields as $k => $collisions) {
|
||||
$caseClauses = array();
|
||||
foreach($collisions as $collision) {
|
||||
if(preg_match('/^"([^"]+)"/', $collision, $matches)) {
|
||||
$collisionBase = $matches[1];
|
||||
$collisionClasses = ClassInfo::subclassesFor($collisionBase);
|
||||
$caseClauses[] = "WHEN \"$baseClass\".\"ClassName\" IN ('"
|
||||
. implode("', '", $collisionClasses) . "') THEN $collision";
|
||||
} else {
|
||||
user_error("Bad collision item '$collision'", E_USER_WARNING);
|
||||
}
|
||||
}
|
||||
$query->select[$k] = "CASE " . implode( " ", $caseClauses) . " ELSE NULL END"
|
||||
. " AS \"$k\"";
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
$query->select[] = "\"$baseClass\".\"ID\"";
|
||||
$query->select[] = "CASE WHEN \"$baseClass\".\"ClassName\" IS NOT NULL THEN \"$baseClass\".\"ClassName\" ELSE '$baseClass' END AS \"RecordClassName\"";
|
||||
|
@ -18,6 +18,41 @@ class DataObjectTest extends SapphireTest {
|
||||
'DataObjectTest_Player',
|
||||
);
|
||||
|
||||
|
||||
function testDataIntegrityWhenTwoSubclassesHaveSameField() {
|
||||
// Save data into DataObjectTest_SubTeam.SubclassDatabaseField
|
||||
$obj = new DataObjectTest_SubTeam();
|
||||
$obj->SubclassDatabaseField = "obj-SubTeam";
|
||||
$obj->write();
|
||||
|
||||
// Change the class
|
||||
$obj->ClassName = 'OtherSubclassWithSameField';
|
||||
$obj->write();
|
||||
$obj->flushCache();
|
||||
|
||||
|
||||
|
||||
|
||||
// Re-fetch from the database and confirm that the data is sourced from
|
||||
// OtherSubclassWithSameField.SubclassDatabaseField
|
||||
$obj = DataObject::get_by_id('DataObjectTest_Team', $obj->ID);
|
||||
$this->assertNull($obj->SubclassDatabaseField);
|
||||
|
||||
// Confirm that save the object in the other direction.
|
||||
$obj->SubclassDatabaseField = 'obj-Other';
|
||||
$obj->write();
|
||||
|
||||
$obj->ClassName = 'DataObjectTest_SubTeam';
|
||||
$obj->write();
|
||||
$obj->flushCache();
|
||||
|
||||
// If we restore the class, the old value has been lying dormant and will be available again.
|
||||
// NOTE: This behaviour is volatile; we may change this in the future to clear fields that
|
||||
// are no longer relevant when changing ClassName
|
||||
$obj = DataObject::get_by_id('DataObjectTest_Team', $obj->ID);
|
||||
$this->assertEquals('obj-SubTeam', $obj->SubclassDatabaseField);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test deletion of DataObjects
|
||||
* - Deleting using delete() on the DataObject
|
||||
|
Loading…
Reference in New Issue
Block a user