From 0b2ed12c6356e81657ee53c4035d44b7337a01c0 Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Tue, 19 Oct 2010 03:01:51 +0000
Subject: [PATCH] BUGFIX: delete orphaned records from versioned tables when
 updating. #5936 (from r110901)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@112866 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/model/Versioned.php      | 39 +++++++++++++++++++++++++--
 tests/model/VersionedTest.php | 50 ++++++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/core/model/Versioned.php b/core/model/Versioned.php
index 76ac1d0aa..a50ea6af2 100755
--- a/core/model/Versioned.php
+++ b/core/model/Versioned.php
@@ -295,9 +295,9 @@ class Versioned extends DataObjectDecorator {
 					);
 				}
 				
-				// Fix data that lacks the uniqueness constraint (since this was added later and
-				// bugs meant that the constraint was validated)
 				if(DB::getConn()->hasTable("{$table}_versions")) {
+					// Fix data that lacks the uniqueness constraint (since this was added later and
+					// bugs meant that the constraint was validated)
 					$duplications = DB::query("SELECT MIN(\"ID\") AS \"ID\", \"RecordID\", \"Version\" 
 						FROM \"{$table}_versions\" GROUP BY \"RecordID\", \"Version\" 
 						HAVING COUNT(*) > 1");
@@ -308,6 +308,41 @@ class Versioned extends DataObjectDecorator {
 						DB::query("DELETE FROM \"{$table}_versions\" WHERE \"RecordID\" = {$dup['RecordID']}
 							AND \"Version\" = {$dup['Version']} AND \"ID\" != {$dup['ID']}");
 					}
+					
+					// Remove junk which has no data in parent classes. Only needs to run the following
+					// when versioned data is spread over multiple tables					
+					if(!$isRootClass && ($versionedTables = ClassInfo::dataClassesFor($table))) {
+						
+						foreach($versionedTables as $child) {
+							if($table == $child) break; // only need subclasses
+							
+							$count = DB::query("
+								SELECT COUNT(*) FROM \"{$table}_versions\"
+								LEFT JOIN \"{$child}_versions\" 
+									ON \"{$child}_versions\".RecordID = \"{$table}_versions\".RecordID 
+									AND \"{$child}_versions\".Version = \"{$table}_versions\".Version
+								WHERE \"{$child}_versions\".ID IS NULL
+							")->value();
+
+							if($count > 0) {
+								DB::alteration_message("Removing orphaned versioned records", "deleted");
+								
+								$effectedIDs = DB::query("
+									SELECT \"{$table}_versions\".ID FROM \"{$table}_versions\"
+									LEFT JOIN \"{$child}_versions\" 
+										ON \"{$child}_versions\".RecordID = \"{$table}_versions\".RecordID 
+										AND \"{$child}_versions\".Version = \"{$table}_versions\".Version
+									WHERE \"{$child}_versions\".ID IS NULL
+								")->column();
+
+								if(is_array($effectedIDs)) {
+									foreach($effectedIDs as $key => $value) {
+										DB::query("DELETE FROM \"{$table}_versions\" WHERE \"{$table}_versions\".ID = '$value'");
+									}
+								}
+							}
+						}
+					}
 				}
 
 				DB::requireTable("{$table}_versions", $versionFields, $versionIndexes);
diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php
index c83f16530..f47d3d9d0 100644
--- a/tests/model/VersionedTest.php
+++ b/tests/model/VersionedTest.php
@@ -5,8 +5,52 @@ class VersionedTest extends SapphireTest {
 
 	protected $extraDataObjects = array(
 		'VersionedTest_DataObject',
+		'VersionedTest_Subclass'
 	);
 	
+	protected $requiredExtensions = array(
+		"VersionedTest_DataObject" => array('Versioned')
+	);
+
+	function testDeletingOrphanedVersions() {
+		$obj = new VersionedTest_Subclass();
+		$obj->ExtraField = 'Foo'; // ensure that child version table gets written
+		$obj->write();
+		$obj->publish('Stage', 'Live');
+		
+		$obj->ExtraField = 'Bar'; // ensure that child version table gets written
+		$obj->write();
+		$obj->publish('Stage', 'Live');
+	
+		$versions = DB::query("SELECT COUNT(*) FROM \"VersionedTest_Subclass_versions\" WHERE \"RecordID\" = '$obj->ID'")->value();
+	
+		$this->assertGreaterThan(0, $versions, 'At least 1 version exists in the history of the page');
+	
+		// Force orphaning of all versions created earlier, only on parent record.
+		// The child versiones table should still have the correct relationship
+		DB::query("DELETE FROM \"VersionedTest_DataObject_versions\" WHERE \"RecordID\" = $obj->ID");
+		
+		// insert a record with no primary key (ID)
+		DB::query("INSERT INTO \"VersionedTest_DataObject_versions\" (RecordID) VALUES ($obj->ID)");
+	
+		// run the script which should clean that up
+		$obj->augmentDatabase();
+	
+		$versions = DB::query("SELECT COUNT(*) FROM \"VersionedTest_Subclass_versions\" WHERE \"RecordID\" = '$obj->ID'")->value();
+		$this->assertEquals(0, $versions, 'Orphaned versions on child tables are removed');
+		
+		// test that it doesn't delete records that we need
+		$obj->write();
+		$obj->publish('Stage', 'Live');
+	
+		$count = DB::query("SELECT COUNT(*) FROM \"VersionedTest_Subclass_versions\" WHERE \"RecordID\" = '$obj->ID'")->value();
+		$obj->augmentDatabase();
+		
+		$count2 = DB::query("SELECT COUNT(*) FROM \"VersionedTest_Subclass_versions\" WHERE \"RecordID\" = '$obj->ID'")->value();
+		
+		$this->assertEquals($count, $count2);
+	}
+	
 	function testForceChangeUpdatesVersion() {
 		$obj = new VersionedTest_DataObject();
 		$obj->Name = "test";
@@ -21,7 +65,7 @@ class VersionedTest extends SapphireTest {
 			"A object Version is increased when just calling forceChange() without any other changes"
 		);
 	}
-	
+
 	/**
 	 * Test Versioned::get_including_deleted()
 	 */
@@ -202,6 +246,10 @@ class VersionedTest_Subclass extends VersionedTest_DataObject implements TestOnl
 	static $db = array(
 		"ExtraField" => "Varchar",
 	);
+	
+	static $extensions = array(
+		"Versioned('Stage', 'Live')"
+	);
 }
 
 /**