BUGFIX: Created Versioned's Version field as a proper Dataobject field.

API CHANGE: DataObject's internal $this->record array doesn't import null values, so that they don't get written back out.
API CHANGE: DataObject queries explicitly list columns, rather than using *.  This means that extraneous columns won't be included.
MINOR: Updated tests for db abstraction.

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@76372 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/model/DataObject.php

@@ -220,7 +220,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 			}
 		}
 
-		$this->record = $this->original = $record;
+		// Set $this->record to $record, but ignore NULLs
+		$this->record = array();
+		foreach($record as $k => $v) {
+			if($v !== null) $this->record[$k] = $v;
+		}
+		$this->original = $this->record;
 
 		// Keep track of the modification date of all the data sourced to make this page
 		// From this we create a Last-Modified HTTP header
@@ -2024,7 +2029,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		}
 
 		// Remove string-based "constructor-arguments" from the DBField definition
-		return isset($fieldMap[$field]) ? strtok($fieldMap[$field],'(') : null;
+		if(isset($fieldMap[$field])) {
+			if(is_string($fieldMap[$field])) return strtok($fieldMap[$field],'(');
+			else return $fieldMap[$field]['type'];
+		}
 	}
 	
 	/**
@@ -2294,37 +2302,35 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		}
 
 		$baseClass = array_shift($tableClasses);
-		$select = array("\"$baseClass\".*");
 
 		// Build our intial query
-		$query = new SQLQuery($select);
+		$query = new SQLQuery(array());
 		$query->from("\"$baseClass\"");
 		$query->where($filter);
 		$query->orderby($sort);
 		$query->limit($limit);
 
 		// Add SQL for multi-value fields on the base table
-		$databaseFields = $this->databaseFields();
+		$databaseFields = self::database_fields($baseClass);
 		if($databaseFields) foreach($databaseFields as $k => $v) {
-			if(!in_array($k, array('ClassName', 'LastEdited', 'Created'))) {
+			if(!in_array($k, array('ClassName', 'LastEdited', 'Created')) && ClassInfo::classImplements($v, 'CompositeDBField')) {
-				if(ClassInfo::classImplements($v, 'CompositeDBField')) {
+				$this->dbObject($k)->addToQuery($query);
-					$this->dbObject($k)->addToQuery($query);
+			} else {
-				}
+				$query->select[] = "\"$baseClass\".\"$k\"";
 			}
 		}
 		// Join all the tables
 		if($tableClasses && self::$subclass_access) {
 			foreach($tableClasses as $tableClass) {
 				$query->from[$tableClass] = "LEFT JOIN \"$tableClass\" ON \"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"";
-				$query->select[] = "\"$tableClass\".*";
 
 				// Add SQL for multi-value fields
 				$databaseFields = self::database_fields($tableClass);
 				if($databaseFields) foreach($databaseFields as $k => $v) {
-					if(!in_array($k, array('ClassName', 'LastEdited', 'Created'))) {
+					if(ClassInfo::classImplements($v, 'CompositeDBField')) {
-						if(ClassInfo::classImplements($v, 'CompositeDBField')) {
+						singleton($tableClass)->dbObject($k)->addToQuery($query);
-							singleton($tableClass)->dbObject($k)->addToQuery($query);
+					} else {
-						}
+						$query->select[] = "\"$tableClass\".\"$k\"";
 					}
 				}
 			}

core/model/Versioned.php

@@ -58,6 +58,9 @@ class Versioned extends DataObjectDecorator {
 	
 	function extraStatics() {
 		return array(
+			'db' => array(
+				'Version' => 'Int',
+			),
 			'has_many' => array(
 				'Versions' => 'SiteTree',
 			)
@@ -188,12 +191,14 @@ class Versioned extends DataObjectDecorator {
 					}
 	
 					// Version fields on each root table (including Stage)
+					/*
 					if(isset($rootTable)) {
 						$stageTable = ($stage == $this->defaultStage) ? $table : "{$table}_$stage";
 						$parts=Array('datatype'=>'int', 'precision'=>11, 'null'=>'not null', 'default'=>(int)0);
 						$values=Array('type'=>'int', 'parts'=>$parts);
 						DB::requireField($stageTable, 'Version', $values);
 					}
+					*/
 				}
 				
 				// Create table for all versions

tests/DataObjectDecoratorTest.php

@@ -71,15 +71,15 @@ class DataObjectDecoratorTest extends SapphireTest {
 	function testDbObjectOnDecoratedFields() {
 		$member = $this->objFromFixture('DataObjectDecoratorTest_Member', 'member1');
 		$this->assertNotNull($member->dbObject('Website'));
-		$this->assertType('Text', $member->dbObject('Website'));
+		$this->assertType('Varchar', $member->dbObject('Website'));
 	}	
 }
 
 class DataObjectDecoratorTest_Member extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"Name" => "Text",
+		"Name" => "Varchar",
-		"Email" => "Text"
+		"Email" => "Varchar"
 	);
 	
 }
@@ -89,7 +89,7 @@ class DataObjectDecoratorTest_ContactRole extends DataObjectDecorator implements
 	function extraStatics() {
 		return array(
 			'db' => array(
-				'Website' => 'Text',
+				'Website' => 'Varchar',
 				'Phone' => 'Varchar(255)',
 			),
 			'has_many' => array(
@@ -106,8 +106,8 @@ class DataObjectDecoratorTest_ContactRole extends DataObjectDecorator implements
 class DataObjectDecoratorTest_RelatedObject extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"FieldOne" => "Text",
+		"FieldOne" => "Varchar",
-		"FieldTwo" => "Text"
+		"FieldTwo" => "Varchar"
 	);
 	
 	static $has_one = array(

tests/DataObjectTest.php

@@ -504,11 +504,14 @@ class DataObjectTest extends SapphireTest {
 	
 	public function testForceInsert() {	
 		/* If you set an ID on an object and pass forceInsert = true, then the object should be correctly created */
+		$conn = DB::getConn();
+		if($conn->hasMethod('allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing('DataObjectTest_Team', true);
 		$obj = new DataObjectTest_SubTeam();
 		$obj->ID = 1001;
 		$obj->Title = 'asdfasdf';
 		$obj->SubclassDatabaseField = 'asdfasdf';
 		$obj->write(false, true);
+		if($conn->hasMethod('allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing('DataObjectTest_Team', false);
 
 		$this->assertEquals("DataObjectTest_SubTeam", DB::query("SELECT \"ClassName\" FROM \"DataObjectTest_Team\" WHERE \"ID\" = $obj->ID")->value());
 
@@ -640,8 +643,8 @@ class DataObjectTest_Player extends Member implements TestOnly {
 class DataObjectTest_Team extends DataObject implements TestOnly {
 
 	static $db = array(
-		'Title' => 'Text', 
+		'Title' => 'Varchar', 
-		'DatabaseField' => 'Text'
+		'DatabaseField' => 'Varchar'
 	);
 
 	static $has_one = array(
@@ -667,15 +670,15 @@ class DataObjectTest_Team extends DataObject implements TestOnly {
 
 class DataObjectTest_FunnyFieldNames extends DataObject implements TestOnly {
 	static $db = array(
-		'Data' => 'Text',
+		'Data' => 'Varchar',
-		'Duplicate' => 'Text',
+		'Duplicate' => 'Varchar',
-		'DbObject' => 'Text',
+		'DbObject' => 'Varchar',
 	);
 }
 
 class DataObjectTest_WithDefaults extends DataObject implements TestOnly {
 	static $db = array(
-		'MyField' => 'Text',
+		'MyField' => 'Varchar',
 	);
 	
 	static $defaults = array(
@@ -685,7 +688,7 @@ class DataObjectTest_WithDefaults extends DataObject implements TestOnly {
 
 class DataObjectTest_SubTeam extends DataObjectTest_Team implements TestOnly {
 	static $db = array(
-		'SubclassDatabaseField' => 'Text'
+		'SubclassDatabaseField' => 'Varchar'
 	);
 }
 
@@ -701,7 +704,7 @@ class DataObjectTest_Team_Decorator extends DataObjectDecorator implements TestO
 	function extraStatics() {
 		return array(
 			'db' => array(
-				'DecoratedDatabaseField' => 'Text'
+				'DecoratedDatabaseField' => 'Varchar'
 			),
 			'has_one' => array(
 				'DecoratedHasOneRelationship' => 'DataObjectTest_Player'

tests/RedirectorPageTest.yml

@@ -12,7 +12,7 @@ RedirectorPage:
    goodinternal:
       Title: Good Internal
       URLSegment: good-internal
-      RedirectionType: Internal:
+      RedirectionType: Internal
       LinkTo: =>Page.dest
    badexternal:
       Title: Bad External

tests/SearchContextTest.php

@@ -126,10 +126,10 @@ class SearchContextTest extends SapphireTest {
 class SearchContextTest_Person extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"Name" => "Text",
+		"Name" => "Varchar",
-		"Email" => "Text",
+		"Email" => "Varchar",
-		"HairColor" => "Text",
+		"HairColor" => "Varchar",
-		"EyeColor" => "Text"
+		"EyeColor" => "Varchar"
 	);
 	
 	static $searchable_fields = array(
@@ -141,8 +141,8 @@ class SearchContextTest_Person extends DataObject implements TestOnly {
 class SearchContextTest_Book extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"Title" => "Text",
+		"Title" => "Varchar",
-		"Summary" => "Text"
+		"Summary" => "Varchar"
 	);
 	
 }
@@ -150,8 +150,8 @@ class SearchContextTest_Book extends DataObject implements TestOnly {
 class SearchContextTest_Company extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"Name" => "Text",
+		"Name" => "Varchar",
-		"Industry" => "Text",
+		"Industry" => "Varchar",
 		"AnnualProfit" => "Int"
 	);
 	
@@ -176,7 +176,7 @@ class SearchContextTest_Company extends DataObject implements TestOnly {
 class SearchContextTest_Project extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"Name" => "Text"
+		"Name" => "Varchar"
 	);
 	
 	static $has_one = array(
@@ -211,7 +211,7 @@ class SearchContextTest_Action extends DataObject implements TestOnly {
 	
 	static $db = array(
 		"Description" => "Text",
-		"SolutionArea" => "Text"
+		"SolutionArea" => "Varchar"
 	);
 	
 	static $has_one = array(
@@ -223,14 +223,14 @@ class SearchContextTest_Action extends DataObject implements TestOnly {
 class SearchContextTest_AllFilterTypes extends DataObject implements TestOnly {
 	
 	static $db = array(
-		"ExactMatch" => "Text",
+		"ExactMatch" => "Varchar",
-		"PartialMatch" => "Text",
+		"PartialMatch" => "Varchar",
-		"Negation" => "Text",
+		"Negation" => "Varchar",
-		"SubstringMatch" => "Text",
+		"SubstringMatch" => "Varchar",
-		"CollectionMatch" => "Text",
+		"CollectionMatch" => "Varchar",
-		"StartsWith" => "Text",
+		"StartsWith" => "Varchar",
-		"EndsWith" => "Text",
+		"EndsWith" => "Varchar",
-		"HiddenValue" => "Text"
+		"HiddenValue" => "Varchar"
 	);
 	
 	static $searchable_fields = array(

tests/SiteTreeActionsTest.php

@@ -71,7 +71,7 @@ if(class_exists('SiteTreeCMSWorkflow')) {
 			$page->deleteFromStage('Stage');
 			
 			// Get the live version of the page
-			$page = Versioned::get_one_by_stage("SiteTree", "Live", "`SiteTree`.ID = $pageID");
+			$page = Versioned::get_one_by_stage("SiteTree", "Live", "\"SiteTree\".ID = $pageID");
 			
 			$author = $this->objFromFixture('Member', 'cmseditor');
 			$this->session()->inst_set('loggedInAs', $author->ID);

tests/SiteTreeTest.php

@@ -198,7 +198,7 @@ class SiteTreeTest extends SapphireTest {
 		Versioned::reading_stage('Live');
 		$deletedPage = Versioned::get_latest_version('SiteTree', $page2ID);
 		$deletedPage->doRestoreToStage();
-		$this->assertTrue(!Versioned::get_one_by_stage("Page", "Live", "`SiteTree`.ID = " . $page2ID));
+		$this->assertTrue(!Versioned::get_one_by_stage("Page", "Live", "\"SiteTree\".ID = " . $page2ID));
 
 		Versioned::reading_stage('Stage');
 		$requeriedPage = DataObject::get_by_id("Page", $page2ID);