Browse files

BUGFIX: delete orphaned records from versioned tables when updating. …

…#5936

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@110901 467b73ca-7a2a-4603-9d3b-597d59a354a9
  • Loading branch information...
1 parent 139ab46 commit f9a84974b687f616c9afc1a70e181b9ba1808511 Will Rossiter committed with sminnee Sep 17, 2010
Showing with 86 additions and 3 deletions.
  1. +37 −2 core/model/Versioned.php
  2. +49 −1 tests/model/VersionedTest.php
View
39 core/model/Versioned.php
@@ -295,9 +295,9 @@ function augmentDatabase() {
);
}
- // 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 @@ function augmentDatabase() {
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);
View
50 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 @@ function testForceChangeUpdatesVersion() {
"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')"
+ );
}
/**

0 comments on commit f9a8497

Please sign in to comment.