From eb583c5f1450e272b1b3c91eef36f255ced7e382 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Sun, 21 Apr 2013 13:18:18 +1200 Subject: [PATCH 1/2] NEW: Added DataObject::getQueriedDatabaseFields() as faster alternative to toMap() API: CompositeDBField::setValue() may be passed an object as its second argument, in addition to array. These changes provide a 15% - 20% performance improvement, and as such justify an small API change in the 3.0 branch. It will likely affect anyone who has created their own composite fields, which is fortunately not all that common. --- docs/en/changelogs/3.0.6.md | 5 +++++ model/DataObject.php | 12 ++++++++++++ model/fieldtypes/CompositeDBField.php | 2 +- model/fieldtypes/Money.php | 5 +++++ view/ViewableData.php | 2 +- 5 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 docs/en/changelogs/3.0.6.md diff --git a/docs/en/changelogs/3.0.6.md b/docs/en/changelogs/3.0.6.md new file mode 100644 index 000000000..7e6448cf0 --- /dev/null +++ b/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. diff --git a/model/DataObject.php b/model/DataObject.php index 421ba8b4a..a1f81ce34 100644 --- a/model/DataObject.php +++ b/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. * diff --git a/model/fieldtypes/CompositeDBField.php b/model/fieldtypes/CompositeDBField.php index 2cdf1808c..979bba6b9 100644 --- a/model/fieldtypes/CompositeDBField.php +++ b/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. diff --git a/model/fieldtypes/Money.php b/model/fieldtypes/Money.php index 30aeb5a44..42ef083c6 100644 --- a/model/fieldtypes/Money.php +++ b/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); diff --git a/view/ViewableData.php b/view/ViewableData.php index 97fb99eb5..d2bc75726 100644 --- a/view/ViewableData.php +++ b/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; } From 9ba26a04c4d7c050022b13505023a58634197fa8 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Mon, 22 Apr 2013 11:50:57 +1200 Subject: [PATCH 2/2] Added test for lazy-loading edge-case in Money field. --- tests/model/MoneyTest.php | 16 ++++++++++++++++ tests/model/MoneyTest.yml | 6 +++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/model/MoneyTest.php b/tests/model/MoneyTest.php index 9b8bfc92d..f9a547cc6 100644 --- a/tests/model/MoneyTest.php +++ b/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', + ); + +} diff --git a/tests/model/MoneyTest.yml b/tests/model/MoneyTest.yml index 1e49e6e35..816f4f9e3 100644 --- a/tests/model/MoneyTest.yml +++ b/tests/model/MoneyTest.yml @@ -1,4 +1,8 @@ MoneyTest_DataObject: test1: MyMoneyCurrency: EUR - MyMoneyAmount: 1.23 \ No newline at end of file + MyMoneyAmount: 1.23 +MoneyTest_SubClass: + test2: + MyOtherMoneyCurrency: GBP + MyOtherMoneyAmount: 2.46 \ No newline at end of file