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')" + ); } /**