diff --git a/model/ManyManyList.php b/model/ManyManyList.php index 0f3b789ce..58e437cfd 100644 --- a/model/ManyManyList.php +++ b/model/ManyManyList.php @@ -8,23 +8,40 @@ */ class ManyManyList extends RelationList { + /** + * @var string $joinTable + */ protected $joinTable; + /** + * @var string $localKey + */ protected $localKey; + /** + * @var string $foreignKey + */ protected $foreignKey; + /** + * @var array $extraFields + */ protected $extraFields; + /** + * @var array $_compositeExtraFields + */ + protected $_compositeExtraFields = array(); + /** * Create a new ManyManyList object. * - * A ManyManyList object represents a list of DataObject records that correspond to a many-many - * relationship. In addition to, + * A ManyManyList object represents a list of {@link DataObject} records + * that correspond to a many-many relationship. * - * Generation of the appropriate record set is left up to the caller, using the normal - * {@link DataList} methods. Addition arguments are used to support {@@link add()} - * and {@link remove()} methods. + * Generation of the appropriate record set is left up to the caller, using + * the normal {@link DataList} methods. Addition arguments are used to + * support {@@link add()} and {@link remove()} methods. * * @param string $dataClass The class of the DataObjects that this will list. * @param string $joinTable The name of the table whose entries define the content of this many_many relation. @@ -36,6 +53,7 @@ class ManyManyList extends RelationList { */ public function __construct($dataClass, $joinTable, $localKey, $foreignKey, $extraFields = array()) { parent::__construct($dataClass); + $this->joinTable = $joinTable; $this->localKey = $localKey; $this->foreignKey = $foreignKey; @@ -46,12 +64,90 @@ class ManyManyList extends RelationList { // Join to the many-many join table $this->dataQuery->innerJoin($joinTable, "\"$joinTable\".\"$this->localKey\" = \"$baseClass\".\"ID\""); - // Query the extra fields from the join table - if($extraFields) $this->dataQuery->selectFromTable($joinTable, array_keys($extraFields)); + // Add the extra fields to the query + if($this->extraFields) { + $this->appendExtraFieldsToQuery(); + } } /** - * Return a filter expression for when getting the contents of the relationship for some foreign ID + * Adds the many_many_extraFields to the select of the underlying + * {@link DataQuery}. + * + * @return void + */ + protected function appendExtraFieldsToQuery() { + $finalized = array(); + + foreach($this->extraFields as $field => $spec) { + $obj = Object::create_from_string($spec); + + if($obj instanceof CompositeDBField) { + $this->_compositeExtraFields[$field] = array(); + + // append the composite field names to the select + foreach($obj->compositeDatabaseFields() as $k => $f) { + $col = $field . $k; + $finalized[] = $col; + + // cache + $this->_compositeExtraFields[$field][] = $k; + } + } else { + $finalized[] = $field; + } + } + + $this->dataQuery->selectFromTable($this->joinTable, $finalized); + } + + /** + * Create a DataObject from the given SQL row. + * + * @param array $row + * @return DataObject + */ + protected function createDataObject($row) { + // remove any composed fields + $add = array(); + + if($this->_compositeExtraFields) { + foreach($this->_compositeExtraFields as $fieldName => $composed) { + // convert joined extra fields into their composite field + // types. + $value = array(); + + foreach($composed as $i => $k) { + if(isset($row[$fieldName . $k])) { + $value[$k] = $row[$fieldName . $k]; + + // don't duplicate data in the record + unset($row[$fieldName . $k]); + } + } + + $obj = Object::create_from_string($this->extraFields[$fieldName], $fieldName); + $obj->setValue($value, null, false); + + $add[$fieldName] = $obj; + } + } + + $dataObject = parent::createDataObject($row); + + foreach($add as $fieldName => $obj) { + $dataObject->$fieldName = $obj; + } + + return $dataObject; + } + + /** + * Return a filter expression for when getting the contents of the + * relationship for some foreign ID + * + * @param int $id + * * @return string */ protected function foreignIDFilter($id = null) { @@ -81,16 +177,23 @@ class ManyManyList extends RelationList { } /** - * Add an item to this many_many relationship - * Does so by adding an entry to the joinTable. - * @param $extraFields A map of additional columns to insert into the joinTable + * Add an item to this many_many relationship. Does so by adding an entry + * to the joinTable. + * + * @param mixed $item + * @param array $extraFields A map of additional columns to insert into the + * joinTable */ public function add($item, $extraFields = null) { - if(is_numeric($item)) $itemID = $item; - else if($item instanceof $this->dataClass) $itemID = $item->ID; - else { - throw new InvalidArgumentException("ManyManyList::add() expecting a $this->dataClass object, or ID value", - E_USER_ERROR); + if(is_numeric($item)) { + $itemID = $item; + } else if($item instanceof $this->dataClass) { + $itemID = $item->ID; + } else { + throw new InvalidArgumentException( + "ManyManyList::add() expecting a $this->dataClass object, or ID value", + E_USER_ERROR + ); } $foreignIDs = $this->getForeignID(); @@ -112,6 +215,7 @@ class ManyManyList extends RelationList { // Insert or update foreach((array)$foreignIDs as $foreignID) { $manipulation = array(); + if($hasExisting) { $manipulation[$this->joinTable]['command'] = 'update'; $manipulation[$this->joinTable]['where'] = "\"$this->joinTable\".\"$this->foreignKey\" = " . @@ -121,9 +225,32 @@ class ManyManyList extends RelationList { $manipulation[$this->joinTable]['command'] = 'insert'; } - if($extraFields) foreach($extraFields as $k => $v) { - if(is_null($v)) $manipulation[$this->joinTable]['fields'][$k] = 'NULL'; - else $manipulation[$this->joinTable]['fields'][$k] = "'" . Convert::raw2sql($v) . "'"; + if($extraFields) { + foreach($extraFields as $k => $v) { + if(is_null($v)) { + $manipulation[$this->joinTable]['fields'][$k] = 'NULL'; + } + else { + if(is_object($v) && $v instanceof DBField) { + // rely on writeToManipulation to manage the changes + // required for this field. + $working = array('fields' => array()); + + // create a new instance of the field so we can + // modify the field name to the correct version. + $field = DBField::create_field(get_class($v), $v); + $field->setName($k); + + $field->writeToManipulation($working); + + foreach($working['fields'] as $extraK => $extraV) { + $manipulation[$this->joinTable]['fields'][$extraK] = $extraV; + } + } else { + $manipulation[$this->joinTable]['fields'][$k] = "'" . Convert::raw2sql($v) . "'"; + } + } + } } $manipulation[$this->joinTable]['fields'][$this->localKey] = $itemID; @@ -135,8 +262,11 @@ class ManyManyList extends RelationList { /** * Remove the given item from this list. - * Note that for a ManyManyList, the item is never actually deleted, only the join table is affected - * @param $itemID The ID of the item to remove. + * + * Note that for a ManyManyList, the item is never actually deleted, only + * the join table is affected. + * + * @param DataObject $item */ public function remove($item) { if(!($item instanceof $this->dataClass)) { @@ -148,8 +278,11 @@ class ManyManyList extends RelationList { /** * Remove the given item from this list. - * Note that for a ManyManyList, the item is never actually deleted, only the join table is affected - * @param $itemID The item it + * + * Note that for a ManyManyList, the item is never actually deleted, only + * the join table is affected + * + * @param int $itemID The item ID */ public function removeByID($itemID) { if(!is_numeric($itemID)) throw new InvalidArgumentException("ManyManyList::removeById() expecting an ID"); @@ -168,7 +301,10 @@ class ManyManyList extends RelationList { } /** - * Remove all items from this many-many join. To remove a subset of items, filter it first. + * Remove all items from this many-many join. To remove a subset of items, + * filter it first. + * + * @return void */ public function removeAll() { $base = ClassInfo::baseDataClass($this->dataClass()); @@ -184,7 +320,9 @@ class ManyManyList extends RelationList { unset($from[$this->joinTable]); $query->setFrom($from); $query->setDistinct(false); - $query->setOrderBy(null, null); // ensure any default sorting is removed, ORDER BY can break DELETE clauses + + // ensure any default sorting is removed, ORDER BY can break DELETE clauses + $query->setOrderBy(null, null); // Use a sub-query as SQLite does not support setting delete targets in // joined queries. @@ -197,16 +335,15 @@ class ManyManyList extends RelationList { } /** - * Find the extra field data for a single row of the relationship - * join table, given the known child ID. - * - * @todo Add tests for this / refactor it / something + * Find the extra field data for a single row of the relationship join + * table, given the known child ID. * * @param string $componentName The name of the component * @param int $itemID The ID of the child for the relationship + * * @return array Map of fieldName => fieldValue */ - function getExtraData($componentName, $itemID) { + public function getExtraData($componentName, $itemID) { $result = array(); if(!is_numeric($itemID)) { diff --git a/model/fieldtypes/DBField.php b/model/fieldtypes/DBField.php index 036745960..6f6317cfa 100644 --- a/model/fieldtypes/DBField.php +++ b/model/fieldtypes/DBField.php @@ -1,14 +1,19 @@ Multi-value DBField objects * - * Sometimes you will want to make DBField classes that don't have a 1-1 match to database fields. To do this, there - * are a number of fields for you to overload. - * - Overload {@link writeToManipulation} to add the appropriate references to the INSERT or UPDATE command - * - Overload {@link addToQuery} to add the appropriate items to a SELECT query's field list + * Sometimes you will want to make DBField classes that don't have a 1-1 match + * to database fields. To do this, there are a number of fields for you to + * overload: + * + * - Overload {@link writeToManipulation} to add the appropriate references to + * the INSERT or UPDATE command + * - Overload {@link addToQuery} to add the appropriate items to a SELECT + * query's field list * - Add appropriate accessor methods * * Subclass Example @@ -68,29 +73,42 @@ abstract class DBField extends ViewableData { /** * Create a DBField object that's not bound to any particular field. + * * Useful for accessing the classes behaviour for other parts of your code. */ public static function create_field($className, $value, $name = null, $object = null) { $dbField = Object::create($className, $name, $object); $dbField->setValue($value, null, false); + return $dbField; } /** * Set the name of this field. - * The name should never be altered, but it if was never given a name in the first place you can set a name. + * + * The name should never be altered, but it if was never given a name in + * the first place you can set a name. + * * If you try an alter the name a warning will be thrown. + * + * @param string $name + * + * @return DBField */ public function setName($name) { - if($this->name) { + if($this->name && $this->name !== $name) { user_error("DBField::setName() shouldn't be called once a DBField already has a name." . "It's partially immutable - it shouldn't be altered after it's given a value.", E_USER_WARNING); } + $this->name = $name; + + return $this; } /** * Returns the name of this field. + * * @return string */ public function getName() { @@ -99,6 +117,7 @@ abstract class DBField extends ViewableData { /** * Returns the value of this field. + * * @return mixed */ public function getValue() { @@ -107,8 +126,8 @@ abstract class DBField extends ViewableData { /** * Set the value on the field. - * Optionally takes the whole record as an argument, - * to pick other values. + * + * Optionally takes the whole record as an argument, to pick other values. * * @param mixed $value * @param array $record @@ -119,9 +138,8 @@ abstract class DBField extends ViewableData { /** - * Determines if the field has a value which - * is not considered to be 'null' in - * a database context. + * Determines if the field has a value which is not considered to be 'null' + * in a database context. * * @return boolean */ @@ -130,9 +148,8 @@ abstract class DBField extends ViewableData { } /** - * Return an encoding of the given value suitable - * for inclusion in a SQL statement. If necessary, - * this should include quotes. + * Return an encoding of the given value suitable for inclusion in a SQL + * statement. If necessary, this should include quotes. * * @param $value mixed The value to check * @return string The encoded value diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index d610ab293..40622e05b 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -1,22 +1,49 @@ write(); + + $money = new Money(); + $money->setAmount(100); + $money->setCurrency('USD'); + + // the actual test is that this does not generate an error in the sql. + $obj->Clients()->add($obj, array( + 'Worth' => $money, + 'Reference' => 'Foo' + )); + + $check = $obj->Clients()->First(); + + $this->assertEquals('Foo', $check->Reference, 'Basic scalar fields should exist'); + $this->assertInstanceOf('Money', $check->Worth, 'Composite fields should exist on the record'); + $this->assertEquals(100, $check->Worth->getAmount()); + } + public function testCreateList() { $list = ManyManyList::create('DataObjectTest_Team','DataObjectTest_Team_Players', 'DataObjectTest_TeamID', 'DataObjectTest_PlayerID'); $this->assertEquals(2, $list->count()); } + public function testRelationshipEmptyOnNewRecords() { // Relies on the fact that (unrelated) teams exist in the fixture file already $newPlayer = new DataObjectTest_Player(); // many_many Teams @@ -210,4 +237,55 @@ class ManyManyListTest extends SapphireTest { $this->assertNotNull(DataObjectTest_Player::get()->byID($b->ID)); } + public function testAppendExtraFieldsToQuery() { + $list = new ManyManyList( + 'ManyManyListTest_ExtraFields', + 'ManyManyListTest_ExtraFields_Clients', + 'ManyManyListTest_ExtraFieldsID', + 'ChildID', array( + 'Worth' => 'Money', + 'Reference' => 'Varchar' + ) + ); + + // ensure that ManyManyListTest_ExtraFields_Clients.ValueCurrency is + // selected. + $db = DB::getConn(); + $expected = 'SELECT DISTINCT "ManyManyListTest_ExtraFields_Clients"."WorthCurrency",' + .' "ManyManyListTest_ExtraFields_Clients"."WorthAmount", "ManyManyListTest_ExtraFields_Clients"."Reference",' + .' "ManyManyListTest_ExtraFields"."ClassName", "ManyManyListTest_ExtraFields"."Created",' + .' "ManyManyListTest_ExtraFields"."LastEdited", "ManyManyListTest_ExtraFields"."ID",' + .' CASE WHEN "ManyManyListTest_ExtraFields"."ClassName" IS NOT NULL THEN' + .' "ManyManyListTest_ExtraFields"."ClassName" ELSE '. $db->prepStringForDB('ManyManyListTest_ExtraFields') + .' END AS "RecordClassName" FROM "ManyManyListTest_ExtraFields" INNER JOIN' + .' "ManyManyListTest_ExtraFields_Clients" ON' + .' "ManyManyListTest_ExtraFields_Clients"."ManyManyListTest_ExtraFieldsID" =' + .' "ManyManyListTest_ExtraFields"."ID"'; + + $this->assertEquals($expected, $list->sql()); + } + + +} + +/** + * @package framework + * @subpackage tests + */ +class ManyManyListTest_ExtraFields extends DataObject implements TestOnly { + + private static $many_many = array( + 'Clients' => 'ManyManyListTest_ExtraFields' + ); + + private static $belongs_many_many = array( + 'WorksWith' => 'ManyManyListTest_ExtraFields' + ); + + private static $many_many_extraFields = array( + 'Clients' => array( + 'Reference' => 'Varchar', + 'Worth' => 'Money' + ) + ); }