From fa8c4b128ca3449175528d8c1814cd9d212b9826 Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Mon, 12 Apr 2010 22:30:07 +0000
Subject: [PATCH] BUGFIX: Fixed potential data corruption issue when you are
 changing the class of a SiteTree subclass between two subclasses that share a
 fieldname. (from r97594)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@102514 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/model/DataObject.php | 37 +++++++++++++++++++++++++++++++++++--
 tests/DataObjectTest.php  | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/core/model/DataObject.php b/core/model/DataObject.php
index b13b06b56..b604ad854 100755
--- a/core/model/DataObject.php
+++ b/core/model/DataObject.php
@@ -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\"";
diff --git a/tests/DataObjectTest.php b/tests/DataObjectTest.php
index aed73a8d5..0b4024d7f 100755
--- a/tests/DataObjectTest.php
+++ b/tests/DataObjectTest.php
@@ -17,6 +17,40 @@ class DataObjectTest extends SapphireTest {
 		'DataObjectTest_ValidatedObject',
 		'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