Merge pull request #1788 from sminnee/perf-fix-in-viewabledata-obj

Performance fix in calls to CompositeDBField::setValue()

docs/en/changelogs/3.0.6.md

@@ -0,0 +1,5 @@
+# 3.0.6 (Not yet released)
+
+## Upgrading
+
+ * If you have created your own composite database fields, then you shoulcd amend the setValue() to allow the passing of an object (usually DataObject) as well as an array.

model/DataObject.php

@@ -730,6 +730,18 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		return $this->record;
 	}
 
+	/**
+	 * Return all currently fetched database fields.
+	 *
+	 * This function is similar to toMap() but doesn't trigger the lazy-loading of all unfetched fields.
+	 * Obviously, this makes it a lot faster.
+	 *
+	 * @return array The data as a map.
+	 */
+	public function getQueriedDatabaseFields() {
+		return $this->record;
+	}
+
 	/**
 	 * Update a number of fields on this object, given a map of the desired changes.
 	 * 

model/fieldtypes/CompositeDBField.php

@@ -118,7 +118,7 @@ interface CompositeDBField {
 	 * parameter. 
 	 * 
 	 * @param DBField|array $value
-	 * @param array $record Map of values loaded from the database
+	 * @param DataObject|array $record An array or object that this field is part of
 	 * @param boolean $markChanged Indicate wether this field should be marked changed. 
 	 *  Set to FALSE if you are initializing this field after construction, rather
 	 *  than setting a new value.

model/fieldtypes/Money.php

@@ -103,6 +103,11 @@ class Money extends DBField implements CompositeDBField {
 	}
 
 	public function setValue($value, $record = null, $markChanged = true) {
+		// Convert an object to an array
+		if($record && $record instanceof DataObject) {
+			$record = $record->getQueriedDatabaseFields();
+		}
+
 		// @todo Allow resetting value to NULL through Money $value field
 		if ($value instanceof Money && $value->exists()) {
 			$this->setCurrency($value->getCurrency(), $markChanged);

tests/model/MoneyTest.php

@@ -17,6 +17,7 @@ class MoneyTest extends SapphireTest {
 
 	protected $extraDataObjects = array(
 		'MoneyTest_DataObject',
+		'MoneyTest_SubClass',
 	);
 	
 	public function testMoneyFieldsReturnedAsObjects() {
@@ -268,6 +269,15 @@ class MoneyTest extends SapphireTest {
 			))->value()
 		);
 	}
+
+	public function testMoneyLazyLoading() {
+		// Get the object, ensuring that MyOtherMoney will be lazy loaded
+		$id = $this->idFromFixture('MoneyTest_SubClass', 'test2');
+		$obj = MoneyTest_DataObject::get()->byID($id);
+
+		$this->assertEquals('£2.46', $obj->obj('MyOtherMoney')->Nice());
+	}
+
 }
 
 class MoneyTest_DataObject extends DataObject implements TestOnly {
@@ -277,3 +287,9 @@ class MoneyTest_DataObject extends DataObject implements TestOnly {
 	);
 
 }
+class MoneyTest_SubClass extends MoneyTest_DataObject implements TestOnly {
+	static $db = array(
+		'MyOtherMoney' => 'Money', 
+	);
+
+}

tests/model/MoneyTest.yml

@@ -1,4 +1,8 @@
 MoneyTest_DataObject:
    test1:
       MyMoneyCurrency: EUR
-      MyMoneyAmount: 1.23
+      MyMoneyAmount: 1.23
+MoneyTest_SubClass:
+   test2:
+      MyOtherMoneyCurrency: GBP
+      MyOtherMoneyAmount: 2.46

view/ViewableData.php

@@ -374,7 +374,7 @@ class ViewableData extends Object implements IteratorAggregate {
 				}
 				
 				$valueObject = Object::create_from_string($castConstructor, $fieldName);
-				$valueObject->setValue($value, ($this->hasMethod('toMap') ? $this->toMap() : null));
+				$valueObject->setValue($value, $this);
 				
 				$value = $valueObject;
 			}