From 50e9eee2e9dcfc5c14608f5e72adf06592b74f7b Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Tue, 2 Jul 2013 13:09:05 +0000 Subject: [PATCH] FIX #2174: SearchFilter needs casting helper for DataObject base fields Commit 964b3f2 fixed an issue where dbObject was returning casting helpers for fields that were not actually DB objects, but had something in $casting config. However, because dbObject was no longer calling DataObject->castingHelper, this exposed a bug that the underlying function db($fieldName) was not returning field specs for the base fields that are created by SS automatically on all DataObjects (i.e. Created, LastEdited, etc). This commit fixes the underlying issue that DataObject->db($fieldName) should return the field specs for *all* DB fields like its documentation says it will, including those base fields that are automatically created and do not appear in $db. --- model/DataObject.php | 41 ++++++++++++++++++++------------ search/filters/SearchFilter.php | 2 +- tests/model/DataListTest.php | 42 ++++++++++++++++++++++++++++++++- tests/model/DataObjectTest.php | 15 +++++++++++- 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/model/DataObject.php b/model/DataObject.php index a0efd74c5..6127e5fb4 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -156,6 +156,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity protected static $_cache_custom_database_fields = array(); protected static $_cache_field_labels = array(); + // base fields which are not defined in static $db + private static $fixed_fields = array( + 'ID' => 'Int', + 'ClassName' => 'Enum', + 'LastEdited' => 'SS_Datetime', + 'Created' => 'SS_Datetime', + ); + /** * Non-static relationship cache, indexed by component name. */ @@ -223,6 +231,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } return array_merge ( + // TODO: should this be using self::$fixed_fields? only difference is ID field + // and ClassName creates an Enum with all values array ( 'ClassName' => self::$classname_spec_cache[$class], 'Created' => 'SS_Datetime', @@ -1651,11 +1661,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Return all of the database fields defined in self::$db and all the parent classes. * Doesn't include any fields specified by self::$has_one. Use $this->has_one() to get these fields + * Also returns "base" fields like "Created", "LastEdited", et cetera. * * @param string $fieldName Limit the output to a specific field name * @return array The database fields */ public function db($fieldName = null) { + if ($fieldName && array_key_exists($fieldName, self::$fixed_fields)) { + return self::$fixed_fields[$fieldName]; + } + $classes = ClassInfo::ancestry($this); $good = false; $items = array(); @@ -1694,6 +1709,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } } + if (!$fieldName) { + // trying to get all fields, so add the fixed fields to return value + $items = array_merge(self::$fixed_fields, $items); + } return $items; } @@ -2387,15 +2406,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return boolean */ public function hasDatabaseField($field) { - // Add base fields which are not defined in static $db - static $fixedFields = array( - 'ID' => 'Int', - 'ClassName' => 'Enum', - 'LastEdited' => 'SS_Datetime', - 'Created' => 'SS_Datetime', - ); - - if(isset($fixedFields[$field])) return true; + if(isset(self::$fixed_fields[$field])) return true; return array_key_exists($field, $this->inheritedDatabaseFields()); } @@ -2658,7 +2669,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Special case for ID field } else if($fieldName == 'ID') { return new PrimaryKey($fieldName, $this); - + + // Special case for ClassName + } else if($fieldName == 'ClassName') { + $val = get_class($this); + return DBField::create_field('Varchar', $val, $fieldName, $this); + // General casting information for items in $db } else if($helper = $this->db($fieldName)) { $obj = Object::create_from_string($helper, $fieldName); @@ -2669,11 +2685,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } else if(preg_match('/ID$/', $fieldName) && $this->has_one(substr($fieldName,0,-2))) { $val = $this->$fieldName; return DBField::create_field('ForeignKey', $val, $fieldName, $this); - - // Special case for ClassName - } else if($fieldName == 'ClassName') { - $val = get_class($this); - return DBField::create_field('Varchar', $val, $fieldName, $this); } } diff --git a/search/filters/SearchFilter.php b/search/filters/SearchFilter.php index 7a02b0e66..c647a3eab 100644 --- a/search/filters/SearchFilter.php +++ b/search/filters/SearchFilter.php @@ -300,4 +300,4 @@ abstract class SearchFilter extends Object { else return null; } -} \ No newline at end of file +} diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index e6e2b9941..456d48120 100644 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -21,7 +21,47 @@ class DataListTest extends SapphireTest { 'DataObjectTest_TeamComment', 'DataObjectTest\NamespacedClass', ); - + + public function testFilterDataObjectByCreatedDate() { + // create an object to test with + $obj1 = new DataObjectTest_ValidatedObject(); + $obj1->Name = 'test obj 1'; + $obj1->write(); + $this->assertTrue($obj1->isInDB()); + + // reload the object from the database and reset its Created timestamp to a known value + $obj1 = DataObjectTest_ValidatedObject::get()->filter(array('Name' => 'test obj 1'))->first(); + $this->assertTrue(is_object($obj1)); + $this->assertEquals('test obj 1', $obj1->Name); + $obj1->Created = '2013-01-01 00:00:00'; + $obj1->write(); + + // reload the object again and make sure that our Created date was properly persisted + $obj1 = DataObjectTest_ValidatedObject::get()->filter(array('Name' => 'test obj 1'))->first(); + $this->assertTrue(is_object($obj1)); + $this->assertEquals('test obj 1', $obj1->Name); + $this->assertEquals('2013-01-01 00:00:00', $obj1->Created); + + // now save a second object to the DB with an automatically-set Created value + $obj2 = new DataObjectTest_ValidatedObject(); + $obj2->Name = 'test obj 2'; + $obj2->write(); + $this->assertTrue($obj2->isInDB()); + + // and a third object + $obj3 = new DataObjectTest_ValidatedObject(); + $obj3->Name = 'test obj 3'; + $obj3->write(); + $this->assertTrue($obj3->isInDB()); + + // now test the filtering based on Created timestamp + $list = DataObjectTest_ValidatedObject::get() + ->filter(array('Created:GreaterThan' => '2013-02-01 00:00:00')) + ->toArray(); + $this->assertEquals(2, count($list)); + + } + public function testSubtract(){ $comment1 = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1'); $subtractList = DataObjectTest_TeamComment::get()->filter('ID', $comment1->ID); diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 534bdbda1..fee16b636 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -18,7 +18,20 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_Player', 'DataObjectTest_TeamComment' ); - + + public function testValidObjectsForBaseFields() { + $obj = new DataObjectTest_ValidatedObject(); + + foreach (array('Created', 'LastEdited', 'ClassName', 'ID') as $field) { + $helper = $obj->dbObject($field); + $this->assertTrue( + ($helper instanceof DBField), + "for {$field} expected helper to be DBField, but was " . + (is_object($helper) ? get_class($helper) : "null") + ); + } + } + public function testDataIntegrityWhenTwoSubclassesHaveSameField() { // Save data into DataObjectTest_SubTeam.SubclassDatabaseField $obj = new DataObjectTest_SubTeam();