FIX: Graceful degradation if obsolete classnames in ChangeSetItem (fixes #6065)

NEW: Add SilverStripe\ORM\UnexpectedDataException class.

This change provides more graceful handling of the case where a
ChangeSetItem is referencing a no-longer-existing class.

The new exception, SilverStripe\ORM\UnexpectedDataException, is
intended to be available for throwing whenever we have unexpected data
in the database.

It can be trapped by the relevant UIs and more graceful errors than
HTTP 500s can be provided.

ORM/UnexpectedDataException.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace SilverStripe\ORM;
+
+/**
+ * Throw this exception whenever unexpected data is found.
+ */
+class UnexpectedDataException extends \Exception
+{
+}

ORM/Versioning/ChangeSet.php

@@ -11,6 +11,7 @@ use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\UnexpectedDataException;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Permission;
 use BadMethodCallException;
@@ -202,15 +203,20 @@ class ChangeSet extends DataObject {
 			$explicit[$explicitKey] = true;
 
 			foreach ($item->findReferenced() as $referee) {
-				/** @var DataObject $referee */
+				try {
-				$key = $this->implicitKey($referee);
+					/** @var DataObject $referee */
+					$key = $this->implicitKey($referee);
 
-				$referenced[$key] = [
+					$referenced[$key] = [
-					'ObjectID' => $referee->ID,
+						'ObjectID' => $referee->ID,
-					'ObjectClass' => $referee->baseClass(),
+						'ObjectClass' => $referee->baseClass(),
-				];
+					];
 
-				$references[$key][] = $item->ID;
+					$references[$key][] = $item->ID;
+
+				// Skip any bad records
+				} catch(UnexpectedDataException $e) {
+				}
 			}
 		}
 

ORM/Versioning/ChangeSetItem.php

@@ -9,6 +9,7 @@ use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\ManyManyList;
 use SilverStripe\ORM\SS_List;
+use SilverStripe\ORM\UnexpectedDataException;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Permission;
 use BadMethodCallException;
@@ -112,6 +113,10 @@ class ChangeSetItem extends DataObject implements Thumbnail {
 	 * @return string
 	 */
 	public function getChangeType() {
+		if(!class_exists($this->ObjectClass)) {
+			throw new UnexpectedDataException("Invalid Class '{$this->ObjectClass}' in ChangeSetItem #{$this->ID}");
+		}
+
 		// Get change versions
 		if($this->VersionBefore || $this->VersionAfter) {
 			$draftVersion = $this->VersionAfter; // After publishing draft was written to stage
@@ -144,6 +149,10 @@ class ChangeSetItem extends DataObject implements Thumbnail {
 	 * @return Versioned|DataObject
 	 */
 	protected function getObjectInStage($stage) {
+		if(!class_exists($this->ObjectClass)) {
+			throw new UnexpectedDataException("Invalid Class '{$this->ObjectClass}' in ChangeSetItem #{$this->ID}");
+		}
+
 		return Versioned::get_by_stage($this->ObjectClass, $stage)->byID($this->ObjectID);
 	}
 
@@ -153,6 +162,10 @@ class ChangeSetItem extends DataObject implements Thumbnail {
 	 * @return Versioned|DataObject
 	 */
 	protected function getObjectLatestVersion() {
+		if(!class_exists($this->ObjectClass)) {
+			throw new UnexpectedDataException("Invalid Class '{$this->ObjectClass}' in ChangeSetItem #{$this->ID}");
+		}
+
 		return Versioned::get_latest_version($this->ObjectClass, $this->ObjectID);
 	}
 
@@ -180,6 +193,10 @@ class ChangeSetItem extends DataObject implements Thumbnail {
 	 * Note: Unlike Versioned::doPublish() and Versioned::doUnpublish, this action is not recursive.
 	 */
 	public function publish() {
+		if(!class_exists($this->ObjectClass)) {
+			throw new UnexpectedDataException("Invalid Class '{$this->ObjectClass}' in ChangeSetItem #{$this->ID}");
+		}
+
 		// Logical checks prior to publish
 		if(!$this->canPublish()) {
 			throw new Exception("The current member does not have permission to publish this ChangeSetItem.");

admin/code/CampaignAdmin.php

@@ -14,6 +14,7 @@ use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\Versioning\ChangeSet;
 use SilverStripe\ORM\Versioning\ChangeSetItem;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\UnexpectedDataException;
 use SilverStripe\Security\SecurityToken;
 use SilverStripe\Security\PermissionProvider;
 use LogicException;
@@ -256,9 +257,6 @@ JSON;
 	 * @return array
 	 */
 	protected function getChangeSetResource(ChangeSet $changeSet) {
-		// Before presenting the changeset to the client,
-		// synchronise it with new changes.
-		$changeSet->sync();
 		$hal = [
 			'_links' => [
 				'self' => [
@@ -267,24 +265,37 @@ JSON;
 			],
 			'ID' => $changeSet->ID,
 			'Name' => $changeSet->Name,
-			'Description' => $changeSet->getDescription(),
 			'Created' => $changeSet->Created,
 			'LastEdited' => $changeSet->LastEdited,
 			'State' => $changeSet->State,
 			'canEdit' => $changeSet->canEdit(),
-			'canPublish' => $changeSet->canPublish(),
+			'canPublish' => false,
 			'_embedded' => ['items' => []]
 		];
-		foreach($changeSet->Changes() as $changeSetItem) {
-			if(!$changeSetItem) {
-				continue;
-			}
 
-			/** @var ChangesetItem $changeSetItem */
+		// Before presenting the changeset to the client,
-			$resource = $this->getChangeSetItemResource($changeSetItem);
+		// synchronise it with new changes.
-			$hal['_embedded']['items'][] = $resource;
+		try {
+			$changeSet->sync();
+			$hal['Description'] = $changeSet->getDescription();
+			$hal['canPublish'] = $changeSet->canPublish();
+
+			foreach($changeSet->Changes() as $changeSetItem) {
+				if(!$changeSetItem) {
+					continue;
+				}
+
+				/** @var ChangesetItem $changeSetItem */
+				$resource = $this->getChangeSetItemResource($changeSetItem);
+				$hal['_embedded']['items'][] = $resource;
+			}
+			$hal['ChangesCount'] = count($hal['_embedded']['items']);
+
+		// An unexpected data exception means that the database is corrupt
+		} catch(UnexpectedDataException $e) {
+			$hal['Description'] = 'Corrupt database! ' . $e->getMessage();
+			$hal['ChangesCount'] = '-';
 		}
-		$hal['ChangesCount'] = count($hal['_embedded']['items']);
 		return $hal;
 	}
 

admin/tests/CampaignAdminTest.php

@@ -0,0 +1,40 @@
+<?php
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Admin\CampaignAdmin;
+use SilverStripe\ORM\Versioning\ChangeSet;
+use SilverStripe\ORM\UnexpectedDataException;
+
+class CampaignAdminTest extends SapphireTest
+{
+
+	/**
+	 * Call a protected method on an object via reflection
+	 *
+	 * @param object $object The object to call the method on
+	 * @param string $method The name of the method
+	 * @param array $args The arguments to pass to the method
+	 */
+	function callProtectedMethod($object, $method, $args = []) {
+	  $class = new ReflectionClass(get_class($object));
+	  $methodObj = $class->getMethod($method);
+	  $methodObj->setAccessible(true);
+	  return $methodObj->invokeArgs($object, $args);
+	}
+
+	function testInvalidDataHandling() {
+		$changeset = new CampaignAdminTest_InvalidChangeSet();
+		$admin = new CampaignAdmin();
+
+		$result = $this->callProtectedMethod($admin, 'getChangeSetResource', [$changeset] );
+		$this->assertEquals('Corrupt database! bad data' , $result['Description']);
+	}
+}
+
+class CampaignAdminTest_InvalidChangeSet extends ChangeSet
+{
+	function sync()
+	{
+		throw new UnexpectedDataException("bad data");
+	}
+}